[clang] [clang-format] Correctly annotate pointer/reference in `if` statement (PR #109370)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

<details>
<summary>Changes</summary>

Fixes #<!-- -->60146.

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


2 Files Affected:

- (modified) clang/lib/Format/TokenAnnotator.cpp (+13-21) 
- (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+14) 


``````````diff
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 6f09835bad3a83..83f5e624d41416 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -300,7 +300,7 @@ class AnnotatingParser {
     return false;
   }
 
-  bool parseParens(bool LookForDecls = false) {
+  bool parseParens(bool IsIf = false) {
     if (!CurrentToken)
       return false;
     assert(CurrentToken->Previous && "Unknown previous token");
@@ -468,24 +468,6 @@ class AnnotatingParser {
         OpeningParen.Previous && OpeningParen.Previous->is(tok::kw_for);
     FormatToken *PossibleObjCForInToken = nullptr;
     while (CurrentToken) {
-      // LookForDecls is set when "if (" has been seen. Check for
-      // 'identifier' '*' 'identifier' followed by not '=' -- this
-      // '*' has to be a binary operator but determineStarAmpUsage() will
-      // categorize it as an unary operator, so set the right type here.
-      if (LookForDecls && CurrentToken->Next) {
-        FormatToken *Prev = CurrentToken->getPreviousNonComment();
-        if (Prev) {
-          FormatToken *PrevPrev = Prev->getPreviousNonComment();
-          FormatToken *Next = CurrentToken->Next;
-          if (PrevPrev && PrevPrev->is(tok::identifier) &&
-              PrevPrev->isNot(TT_TypeName) && Prev->isPointerOrReference() &&
-              CurrentToken->is(tok::identifier) && Next->isNot(tok::equal)) {
-            Prev->setType(TT_BinaryOperator);
-            LookForDecls = false;
-          }
-        }
-      }
-
       if (CurrentToken->Previous->is(TT_PointerOrReference) &&
           CurrentToken->Previous->Previous->isOneOf(tok::l_paren,
                                                     tok::coloncolon)) {
@@ -581,6 +563,15 @@ class AnnotatingParser {
           PossibleObjCForInToken = nullptr;
         }
       }
+      if (IsIf && CurrentToken->is(tok::semi)) {
+        for (auto *Tok = OpeningParen.Next;
+             Tok != CurrentToken &&
+             !Tok->isOneOf(tok::equal, tok::l_paren, tok::l_brace);
+             Tok = Tok->Next) {
+          if (Tok->isPointerOrReference())
+            Tok->setFinalizedType(TT_PointerOrReference);
+        }
+      }
       if (MightBeObjCForRangeLoop && CurrentToken->is(Keywords.kw_in)) {
         PossibleObjCForInToken = CurrentToken;
         PossibleObjCForInToken->setType(TT_ObjCForIn);
@@ -1307,7 +1298,7 @@ class AnnotatingParser {
     // Multi-line string itself is a single annotated token.
     if (Tok->is(TT_TableGenMultiLineString))
       return true;
-    switch (Tok->Tok.getKind()) {
+    switch (bool IsIf = false; Tok->Tok.getKind()) {
     case tok::plus:
     case tok::minus:
       if (!Tok->Previous && Line.MustBeDeclaration)
@@ -1461,11 +1452,12 @@ class AnnotatingParser {
           CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier)) {
         next();
       }
+      IsIf = true;
       [[fallthrough]];
     case tok::kw_while:
       if (CurrentToken && CurrentToken->is(tok::l_paren)) {
         next();
-        if (!parseParens(/*LookForDecls=*/true))
+        if (!parseParens(IsIf))
           return false;
       }
       break;
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 34c03d668a9a0a..32df6dd5fa5bf5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -294,6 +294,20 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   EXPECT_TOKEN(Tokens[7], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[10], tok::greater, TT_BinaryOperator);
 
+  Tokens = annotate("if (Foo *foo; bar)");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("if (Foo **foo(); bar)");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("if (Foo *&foo{a}; bar)");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_PointerOrReference);
+
   FormatStyle Style = getLLVMStyle();
   Style.TypeNames.push_back("MYI");
   Tokens = annotate("if (MYI *p{nullptr})", Style);

``````````

</details>


https://github.com/llvm/llvm-project/pull/109370