[Clang] Repair the function "rParenEndsCast" to make incorrect judgments in template variable cases

[dty2]

Try to fix issue: #120148
I'm a newbie, please give me some suggestions for modification.
Especially I use a global array "castIdentifiers", I feel that this implementation is not good, but I don't know how to modify it.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-vectorizers
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang-format

Author: 执着 (dty2)

Changes

Try to fix issue: #120148
I'm a newbie, please give me some suggestions for modification.
Especially I use a global array "castIdentifiers", I feel that this implementation is not good, but I don't know how to modify it.

---

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

1 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+57-31)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index bc5239209f3aab..14db6e9116ab2e 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -17,6 +17,8 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "format-token-annotator"
@@ -38,6 +40,9 @@ static bool mustBreakAfterAttributes(const FormatToken &Tok,
 
 namespace {
 
+SmallVector<llvm::StringRef, 100> castIdentifiers{"__type_identity_t",
+                                                  "remove_reference_t"};
+
 /// Returns \c true if the line starts with a token that can start a statement
 /// with an initializer.
 static bool startsWithInitStatement(const AnnotatedLine &Line) {
@@ -492,7 +497,8 @@ class AnnotatingParser {
             ProbablyFunctionType && CurrentToken->Next &&
             (CurrentToken->Next->is(tok::l_paren) ||
              (CurrentToken->Next->is(tok::l_square) &&
-              Line.MustBeDeclaration))) {
+              (Line.MustBeDeclaration ||
+               (PrevNonComment && PrevNonComment->isTypeName(LangOpts)))))) {
           OpeningParen.setType(OpeningParen.Next->is(tok::caret)
                                    ? TT_ObjCBlockLParen
                                    : TT_FunctionTypeLParen);
@@ -2403,7 +2409,8 @@ class AnnotatingParser {
       // not auto operator->() -> xxx;
       Current.setType(TT_TrailingReturnArrow);
     } else if (Current.is(tok::arrow) && Current.Previous &&
-               Current.Previous->is(tok::r_brace)) {
+               Current.Previous->is(tok::r_brace) &&
+               Current.Previous->is(BK_Block)) {
       // Concept implicit conversion constraint needs to be treated like
       // a trailing return type  ... } -> <type>.
       Current.setType(TT_TrailingReturnArrow);
@@ -2472,6 +2479,9 @@ class AnnotatingParser {
           Current.getNextNonComment()->isOneOf(tok::comma, tok::r_brace)) {
         Current.setType(TT_StringInConcatenation);
       }
+    } else if (Current.is(tok::kw_using)) {
+      if (Current.Next->Next->Next->isTypeName(LangOpts))
+        castIdentifiers.push_back(Current.Next->TokenText);
     } else if (Current.is(tok::l_paren)) {
       if (lParenStartsCppCast(Current))
         Current.setType(TT_CppCastLParen);
@@ -2829,8 +2839,18 @@ class AnnotatingParser {
         IsQualifiedPointerOrReference(BeforeRParen, LangOpts);
     bool ParensCouldEndDecl =
         AfterRParen->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
-    if (ParensAreType && !ParensCouldEndDecl)
+    if (ParensAreType && !ParensCouldEndDecl) {
+      if (BeforeRParen->is(TT_TemplateCloser)) {
+        auto *Prev = BeforeRParen->MatchingParen->getPreviousNonComment();
+        if (Prev) {
+          for (auto &name : castIdentifiers)
+            if (Prev->TokenText == name)
+              return true;
+          return false;
+        }
+      }
       return true;
+    }
 
     // At this point, we heuristically assume that there are no casts at the
     // start of the line. We assume that we have found most cases where there
@@ -3901,6 +3921,11 @@ bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const {
 }
 
 void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
+  if (Line.Computed)
+    return;
+
+  Line.Computed = true;
+
   for (AnnotatedLine *ChildLine : Line.Children)
     calculateFormattingInformation(*ChildLine);
 
@@ -6105,6 +6130,35 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
       return false;
   }
 
+  // We can break before an r_brace if there was a break after the matching
+  // l_brace, which is tracked by BreakBeforeClosingBrace, or if we are in a
+  // block-indented initialization list.
+  if (Right.is(tok::r_brace)) {
+    return Right.MatchingParen && (Right.MatchingParen->is(BK_Block) ||
+                                   (Right.isBlockIndentedInitRBrace(Style)));
+  }
+
+  // We only break before r_paren if we're in a block indented context.
+  if (Right.is(tok::r_paren)) {
+    if (Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent ||
+        !Right.MatchingParen) {
+      return false;
+    }
+    auto Next = Right.Next;
+    if (Next && Next->is(tok::r_paren))
+      Next = Next->Next;
+    if (Next && Next->is(tok::l_paren))
+      return false;
+    const FormatToken *Previous = Right.MatchingParen->Previous;
+    return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
+  }
+
+  if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
+      Right.is(TT_TrailingAnnotation) &&
+      Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
+    return false;
+  }
+
   if (Left.is(tok::at))
     return false;
   if (Left.Tok.getObjCKeywordID() == tok::objc_interface)
@@ -6260,34 +6314,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
     return false;
   }
 
-  // We only break before r_brace if there was a corresponding break before
-  // the l_brace, which is tracked by BreakBeforeClosingBrace.
-  if (Right.is(tok::r_brace)) {
-    return Right.MatchingParen && (Right.MatchingParen->is(BK_Block) ||
-                                   (Right.isBlockIndentedInitRBrace(Style)));
-  }
-
-  // We only break before r_paren if we're in a block indented context.
-  if (Right.is(tok::r_paren)) {
-    if (Style.AlignAfterOpenBracket != FormatStyle::BAS_BlockIndent ||
-        !Right.MatchingParen) {
-      return false;
-    }
-    auto Next = Right.Next;
-    if (Next && Next->is(tok::r_paren))
-      Next = Next->Next;
-    if (Next && Next->is(tok::l_paren))
-      return false;
-    const FormatToken *Previous = Right.MatchingParen->Previous;
-    return !(Previous && (Previous->is(tok::kw_for) || Previous->isIf()));
-  }
-
-  if (Left.isOneOf(tok::r_paren, TT_TrailingAnnotation) &&
-      Right.is(TT_TrailingAnnotation) &&
-      Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) {
-    return false;
-  }
-
   // Allow breaking after a trailing annotation, e.g. after a method
   // declaration.
   if (Left.is(TT_TrailingAnnotation)) {

[owenca]

You need to add a unit test.

[owenca]

Please run ninja FormatTests before submitting the patch. It fails a few unit tests before crashing.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[HazardyKnusperkeks]

Suggested change

	// TODO: Add new Type modifiers
	// TODO: Add new Type modifiers.

[HazardyKnusperkeks]

Suggested change

	for (auto &name : castIdentifiers)
	if (Prev->TokenText == name)
	return true;
	return false;
	return std::find(castIdentifiers.begin(), castIdentifiers.end(), Prev->TokenText) != castIdentifiers.end();

[owenca]

IMO this is too simplistic to be very useful. See the general syntax here. Also, this doesn't work at all if the using statement is in a header file.

[owenca]

What about std::vector, std::whatever, foo::bar, etc?

[owenca]

See #121318.