[clang-format] Fix a bug in mis-annotating arrows

[owenca]

Fixed #66923.

[llvmbot]

@llvm/pr-subscribers-clang-format

Changes

Fixed #66923.

---

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

2 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+41-31)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+4)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 95d039b459b43e8..907ca4163620643 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2014,8 +2014,7 @@ class AnnotatingParser {
                Style.Language == FormatStyle::LK_Java) {
       Current.setType(TT_LambdaArrow);
     } else if (Current.is(tok::arrow) && AutoFound &&
-               (Line.MightBeFunctionDecl || Line.InPPDirective) &&
-               Current.NestingLevel == 0 &&
+               Line.MightBeFunctionDecl && Current.NestingLevel == 0 &&
                !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
       // not auto operator->() -> xxx;
       Current.setType(TT_TrailingReturnArrow);
@@ -3250,7 +3249,8 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) {
 // This function heuristically determines whether 'Current' starts the name of a
 // function declaration.
 static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
-                                      const AnnotatedLine &Line) {
+                                      const AnnotatedLine &Line,
+                                      FormatToken *&ClosingParen) {
   assert(Current.Previous);
 
   if (Current.is(TT_FunctionDeclarationName))
@@ -3336,16 +3336,16 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
   // Check whether parameter list can belong to a function declaration.
   if (!Next || Next->isNot(tok::l_paren) || !Next->MatchingParen)
     return false;
+  ClosingParen = Next->MatchingParen;
+  assert(ClosingParen->is(tok::r_paren));
   // If the lines ends with "{", this is likely a function definition.
   if (Line.Last->is(tok::l_brace))
     return true;
-  if (Next->Next == Next->MatchingParen)
+  if (Next->Next == ClosingParen)
     return true; // Empty parentheses.
   // If there is an &/&& after the r_paren, this is likely a function.
-  if (Next->MatchingParen->Next &&
-      Next->MatchingParen->Next->is(TT_PointerOrReference)) {
+  if (ClosingParen->Next && ClosingParen->Next->is(TT_PointerOrReference))
     return true;
-  }
 
   // Check for K&R C function definitions (and C++ function definitions with
   // unnamed parameters), e.g.:
@@ -3362,7 +3362,7 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
     return true;
   }
 
-  for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
+  for (const FormatToken *Tok = Next->Next; Tok && Tok != ClosingParen;
        Tok = Tok->Next) {
     if (Tok->is(TT_TypeDeclarationParen))
       return true;
@@ -3434,11 +3434,12 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
     calculateArrayInitializerColumnList(Line);
 
   bool LineIsFunctionDeclaration = false;
+  FormatToken *ClosingParen = nullptr;
   for (FormatToken *Tok = Current, *AfterLastAttribute = nullptr; Tok;
        Tok = Tok->Next) {
     if (Tok->Previous->EndsCppAttributeGroup)
       AfterLastAttribute = Tok;
-    if (isFunctionDeclarationName(Style.isCpp(), *Tok, Line)) {
+    if (isFunctionDeclarationName(Style.isCpp(), *Tok, Line, ClosingParen)) {
       LineIsFunctionDeclaration = true;
       Tok->setFinalizedType(TT_FunctionDeclarationName);
       if (AfterLastAttribute &&
@@ -3450,29 +3451,38 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
     }
   }
 
-  if (Style.isCpp() && !LineIsFunctionDeclaration) {
-    // Annotate */&/&& in `operator` function calls as binary operators.
-    for (const auto *Tok = Line.First; Tok; Tok = Tok->Next) {
-      if (Tok->isNot(tok::kw_operator))
-        continue;
-      do {
-        Tok = Tok->Next;
-      } while (Tok && Tok->isNot(TT_OverloadedOperatorLParen));
-      if (!Tok)
-        break;
-      const auto *LeftParen = Tok;
-      for (Tok = Tok->Next; Tok && Tok != LeftParen->MatchingParen;
-           Tok = Tok->Next) {
-        if (Tok->isNot(tok::identifier))
-          continue;
-        auto *Next = Tok->Next;
-        const bool NextIsBinaryOperator =
-            Next && Next->isOneOf(tok::star, tok::amp, tok::ampamp) &&
-            Next->Next && Next->Next->is(tok::identifier);
-        if (!NextIsBinaryOperator)
+  if (Style.isCpp()) {
+    if (LineIsFunctionDeclaration) {
+      for (auto *Tok = ClosingParen; Tok; Tok = Tok->Next) {
+        if (Tok->is(tok::arrow)) {
+          Tok->setType(TT_TrailingReturnArrow);
+          break;
+        }
+      }
+    } else {
+      // Annotate */&/&& in `operator` function calls as binary operators.
+      for (const auto *Tok = Line.First; Tok; Tok = Tok->Next) {
+        if (Tok->isNot(tok::kw_operator))
           continue;
-        Next->setType(TT_BinaryOperator);
-        Tok = Next;
+        do {
+          Tok = Tok->Next;
+        } while (Tok && Tok->isNot(TT_OverloadedOperatorLParen));
+        if (!Tok)
+          break;
+        const auto *LeftParen = Tok;
+        for (Tok = Tok->Next; Tok && Tok != LeftParen->MatchingParen;
+             Tok = Tok->Next) {
+          if (Tok->isNot(tok::identifier))
+            continue;
+          auto *Next = Tok->Next;
+          const bool NextIsBinaryOperator =
+              Next && Next->isOneOf(tok::star, tok::amp, tok::ampamp) &&
+              Next->Next && Next->Next->is(tok::identifier);
+          if (!NextIsBinaryOperator)
+            continue;
+          Next->setType(TT_BinaryOperator);
+          Tok = Next;
+        }
       }
     }
   }
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 204b00dd09f04d1..2f3233adfb02d26 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1731,6 +1731,10 @@ TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
   ASSERT_EQ(Tokens.size(), 16u) << Tokens;
   EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
 
+  Tokens = annotate("#define P(ptr) auto p = (ptr)->p");
+  ASSERT_EQ(Tokens.size(), 15u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::arrow, TT_Unknown);
+
   // Mixed
   Tokens = annotate("auto f() -> int { auto a = b()->c; }");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;

[petrhosek]

This introduced a regression where Clang-Format misformats Thread Safety Analysis annotations that include an arrow, #69234 has more details. Would it be possible to take a look?

[owenca]

See #69249.