Skip to content
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

[clang-format] Fix a bug in annotating && followed by * or & #65933

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Sep 11, 2023

Fixes #65877.

@owenca owenca requested a review from a team as a code owner September 11, 2023 08:16
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-format labels Sep 11, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 11, 2023

@llvm/pr-subscribers-clang

Changes

Fixes #65877.

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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+5-2)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+6)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index e925bee44cd0c6..142168e074bbc2 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2598,9 +2598,12 @@ class AnnotatingParser {
     if (InTemplateArgument && NextToken->Tok.isAnyIdentifier())
       return TT_BinaryOperator;
 
-    // "&&(" is quite unlikely to be two successive unary "&".
-    if (Tok.is(tok::ampamp) && NextToken->is(tok::l_paren))
+    // "&&" followed by "(", "*", or "&" is quite unlikely to be two successive
+    // unary "&".
+    if (Tok.is(tok::ampamp) &&
+        NextToken->isOneOf(tok::l_paren, tok::star, tok::amp)) {
       return TT_BinaryOperator;
+    }
 
     // This catches some cases where evaluation order is used as control flow:
     //   aaa && aaa->f();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 43485298371294..be025dab86fafa 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -280,6 +280,12 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[27], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("foo = *i < *j && *j > *k;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::greater, TT_BinaryOperator);
+
   FormatStyle Style = getLLVMStyle();
   Style.TypeNames.push_back("MYI");
   Tokens = annotate("if (MYI *p{nullptr})", Style);

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 11, 2023

@llvm/pr-subscribers-clang-format

Changes

Fixes #65877.

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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+5-2)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+6)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index e925bee44cd0c6a..142168e074bbc27 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2598,9 +2598,12 @@ class AnnotatingParser {
     if (InTemplateArgument && NextToken->Tok.isAnyIdentifier())
       return TT_BinaryOperator;
 
-    // "&&(" is quite unlikely to be two successive unary "&".
-    if (Tok.is(tok::ampamp) && NextToken->is(tok::l_paren))
+    // "&&" followed by "(", "*", or "&" is quite unlikely to be two successive
+    // unary "&".
+    if (Tok.is(tok::ampamp) &&
+        NextToken->isOneOf(tok::l_paren, tok::star, tok::amp)) {
       return TT_BinaryOperator;
+    }
 
     // This catches some cases where evaluation order is used as control flow:
     //   aaa && aaa->f();
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 434852983712940..be025dab86fafa5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -280,6 +280,12 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[27], tok::ampamp, TT_BinaryOperator);
 
+  Tokens = annotate("foo = *i < *j && *j > *k;");
+  EXPECT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::less, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[7], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::greater, TT_BinaryOperator);
+
   FormatStyle Style = getLLVMStyle();
   Style.TypeNames.push_back("MYI");
   Tokens = annotate("if (MYI *p{nullptr})", Style);

@owenca owenca changed the title [clang-format] Fix a bug in annotating '&&' enclosed in '<' and '>' [clang-format] Fix a bug in annotating && enclosed in < and > Sep 11, 2023
Copy link
Member

@rymiel rymiel left a comment

Choose a reason for hiding this comment

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

I'm also suspicious of if a dereference like *j should ever be legal in a template argument, but that would be a different case

@owenca owenca changed the title [clang-format] Fix a bug in annotating && enclosed in < and > [clang-format] Fix a bug in annotating && followed by * or & Sep 11, 2023
@owenca owenca merged commit 0b2c88e into llvm:main Sep 14, 2023
2 checks passed
@owenca owenca deleted the determineStarAmpUsage branch September 14, 2023 06:57
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Sep 14, 2023
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
@owenca owenca removed the clang Clang issues not falling into any other category label Mar 3, 2024
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 formatting when expression has template like comparison operators
4 participants