Skip to content

[clang-format] Don't allow casts in front of ampamp #77704

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 1 commit into from
Jan 12, 2024

Conversation

rymiel
Copy link
Member

@rymiel rymiel commented Jan 10, 2024

clang-format performs a heuristic to see if a right parenthesis is the parenthesis of a cast expression. This check never looked ahead to see if the next token is an ampamp && token. This resulted in the paren being set as an CastRParen, and the following ampamp got annotated as an UnaryOperator!

Since && can never be a unary operator is standard C++, this patch forbids the right paren from ever becoming a cast.

Fixes #77680

clang-format performs a heuristic to see if a right parenthesis is the
parenthesis of a cast expression. This check never looked ahead to see
if the next token is an ampamp && token. This resulted in the paren
being set as an CastRParen, and the following ampamp got annotated as an
UnaryOperator!

Since && can never be a unary operator is standard C++, this patch
forbids the right paren from ever becoming a cast.

Fixes llvm#77680
@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2024

@llvm/pr-subscribers-clang-format

Author: Emilia Kond (rymiel)

Changes

clang-format performs a heuristic to see if a right parenthesis is the parenthesis of a cast expression. This check never looked ahead to see if the next token is an ampamp && token. This resulted in the paren being set as an CastRParen, and the following ampamp got annotated as an UnaryOperator!

Since && can never be a unary operator is standard C++, this patch forbids the right paren from ever becoming a cast.

Fixes #77680


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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+1-1)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+5)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 8b43438c72dfe1..854e85db5be4d9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2369,7 +2369,7 @@ class AnnotatingParser {
       }
     }
 
-    if (Tok.Next->is(tok::question))
+    if (Tok.Next->isOneOf(tok::question, tok::ampamp))
       return false;
 
     // `foreach((A a, B b) in someList)` should not be seen as a cast.
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index decc0785c5cde7..4f4c54b344e109 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1066,6 +1066,11 @@ TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   EXPECT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause);
+
+  Tokens = annotate("template <typename T>\n"
+                    "concept C = (!Foo<T>) && Bar;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {

Copy link
Contributor

@mydeveloperday mydeveloperday left a comment

Choose a reason for hiding this comment

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

LGTM

Tokens = annotate("template <typename T>\n"
"concept C = (!Foo<T>) && Bar;");
ASSERT_EQ(Tokens.size(), 19u) << Tokens;
EXPECT_TOKEN(Tokens[15], tok::ampamp, TT_BinaryOperator);
Copy link
Contributor

Choose a reason for hiding this comment

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

out of interest what did it think it was before?

Copy link
Member Author

Choose a reason for hiding this comment

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

TT_UnaryOperator, as outlined in the issue description

@rymiel rymiel merged commit 791637e into llvm:main Jan 12, 2024
@rymiel rymiel deleted the format/cast-ampamp branch January 12, 2024 01:47
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
clang-format performs a heuristic to see if a right parenthesis is the
parenthesis of a cast expression. This check never looked ahead to see
if the next token is an ampamp && token. This resulted in the paren
being set as an CastRParen, and the following ampamp got annotated as an
UnaryOperator!

Since && can never be a unary operator is standard C++, this patch
forbids the right paren from ever becoming a cast.

Fixes llvm#77680
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] parenthesized expression in concept causes && to be misinterpreted as rvalue reference
5 participants