Skip to content

[clang-format] Allow decltype in requires clause #78847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

rymiel
Copy link
Member

@rymiel rymiel commented Jan 20, 2024

If clang-format is not sure whether a requires keyword starts a requires clause or a requires expression, it looks ahead to see if any token disqualifies it from being a requires clause. Among these tokens was decltype, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just means we need more test coverage!

Fixes #78645

If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just means
we need more test coverage!

Fixes llvm#78645
@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2024

@llvm/pr-subscribers-clang-format

Author: Emilia Kond (rymiel)

Changes

If clang-format is not sure whether a requires keyword starts a requires clause or a requires expression, it looks ahead to see if any token disqualifies it from being a requires clause. Among these tokens was decltype, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just means we need more test coverage!

Fixes #78645


Full diff: https://github.com/llvm/llvm-project/pull/78847.diff

2 Files Affected:

  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+1)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+10)
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index c08ce86449b6ea..62a6018580a0d1 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3433,6 +3433,7 @@ bool clang::format::UnwrappedLineParser::parseRequires() {
     case tok::coloncolon:
       LastWasColonColon = true;
       break;
+    case tok::kw_decltype:
     case tok::identifier:
       if (FoundType && !LastWasColonColon && OpenAngles == 0) {
         FormatTok = Tokens->setPosition(StoredPosition);
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 64b2abac5cce53..3f94c464b41343 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1071,6 +1071,16 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
                     "concept C = (!Foo<T>) && Bar;");
   ASSERT_EQ(Tokens.size(), 19u) << Tokens;
   EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("void f() & requires(C<decltype(x)>) {}");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("auto f() -> int& requires(C<decltype(x)>) {}");
+  ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {

@rymiel rymiel marked this pull request as draft January 20, 2024 18:23
@rymiel rymiel marked this pull request as ready for review January 20, 2024 18:24
" return 1;\n"
" }\n"
" return 0;\n"
"}\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the newline?

@owenca
Copy link
Contributor

owenca commented Feb 1, 2024

Should we merge this patch?

@rymiel rymiel merged commit 9b68c09 into llvm:main Feb 1, 2024
@rymiel rymiel deleted the format/requires-clause-decltype branch February 1, 2024 06:00
carlosgalvezp pushed a commit to carlosgalvezp/llvm-project that referenced this pull request Feb 1, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just
means we need more test coverage!

Fixes llvm#78645
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
If clang-format is not sure whether a `requires` keyword starts a
requires clause or a requires expression, it looks ahead to see if any
token disqualifies it from being a requires clause. Among these tokens
was `decltype`, since it fell through the switch.

This patch allows decltype to exist in a require clause.

I'm not 100% sure this change won't have repercussions, but that just
means we need more test coverage!

Fixes llvm#78645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-format] Incorrect method body when combining trailing return type & decltype in requires clause
4 participants