[clang] [clang-format] Handle template closer followed by empty paretheses (PR #110408)

Owen Pan via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 2 18:22:11 PDT 2024


https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/110408

>From 27135c008868cc4f17b7ca5ff9c2af9c2fc02135 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sat, 28 Sep 2024 22:42:56 -0700
Subject: [PATCH 1/2] [clang-format] Handle template closer followed by empty
 paretheses

Fixes #109925.
---
 clang/lib/Format/TokenAnnotator.cpp           | 42 ++++++++++---------
 clang/lib/Format/UnwrappedLineParser.cpp      | 15 ++++---
 clang/unittests/Format/TokenAnnotatorTest.cpp |  7 ++++
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index f665ce2ad81eb0..1da1bb66f84eb0 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -189,25 +189,29 @@ class AnnotatingParser {
       next();
     }
 
-    for (bool SeenTernaryOperator = false; CurrentToken;) {
+    for (bool SeenTernaryOperator = false, MaybeAngles = true; CurrentToken;) {
       const bool InExpr = Contexts[Contexts.size() - 2].IsExpression;
       if (CurrentToken->is(tok::greater)) {
         const auto *Next = CurrentToken->Next;
-        // Try to do a better job at looking for ">>" within the condition of
-        // a statement. Conservatively insert spaces between consecutive ">"
-        // tokens to prevent splitting right bitshift operators and potentially
-        // altering program semantics. This check is overly conservative and
-        // will prevent spaces from being inserted in select nested template
-        // parameter cases, but should not alter program semantics.
-        if (Next && Next->is(tok::greater) &&
-            Left->ParentBracket != tok::less &&
-            CurrentToken->getStartOfNonWhitespace() ==
-                Next->getStartOfNonWhitespace().getLocWithOffset(-1)) {
-          return false;
-        }
-        if (InExpr && SeenTernaryOperator &&
-            (!Next || !Next->isOneOf(tok::l_paren, tok::l_brace))) {
-          return false;
+        if (CurrentToken->isNot(TT_TemplateCloser)) {
+          // Try to do a better job at looking for ">>" within the condition of
+          // a statement. Conservatively insert spaces between consecutive ">"
+          // tokens to prevent splitting right shift operators and potentially
+          // altering program semantics. This check is overly conservative and
+          // will prevent spaces from being inserted in select nested template
+          // parameter cases, but should not alter program semantics.
+          if (Next && Next->is(tok::greater) &&
+              Left->ParentBracket != tok::less &&
+              CurrentToken->getStartOfNonWhitespace() ==
+                  Next->getStartOfNonWhitespace().getLocWithOffset(-1)) {
+            return false;
+          }
+          if (InExpr && SeenTernaryOperator &&
+              (!Next || !Next->isOneOf(tok::l_paren, tok::l_brace))) {
+            return false;
+          }
+          if (!MaybeAngles)
+            return false;
         }
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
@@ -243,11 +247,11 @@ class AnnotatingParser {
       // operator that was misinterpreted because we are parsing template
       // parameters.
       // FIXME: This is getting out of hand, write a decent parser.
-      if (InExpr && !Line.startsWith(tok::kw_template) &&
+      if (MaybeAngles && InExpr && !Line.startsWith(tok::kw_template) &&
           Prev.is(TT_BinaryOperator)) {
         const auto Precedence = Prev.getPrecedence();
         if (Precedence > prec::Conditional && Precedence < prec::Relational)
-          return false;
+          MaybeAngles = false;
       }
       if (Prev.isOneOf(tok::question, tok::colon) && !Style.isProto())
         SeenTernaryOperator = true;
@@ -1614,7 +1618,7 @@ class AnnotatingParser {
         return false;
       break;
     case tok::greater:
-      if (Style.Language != FormatStyle::LK_TextProto)
+      if (Style.Language != FormatStyle::LK_TextProto && Tok->is(TT_Unknown))
         Tok->setType(TT_BinaryOperator);
       if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
         Tok->SpacesRequiredBefore = 1;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 40f77266fabdca..1f8013e3e4858f 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2556,7 +2556,8 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
         parseChildBlock();
       break;
     case tok::r_paren: {
-      const auto *Prev = LeftParen->Previous;
+      auto *Prev = LeftParen->Previous;
+      const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
       if (!MightBeStmtExpr && !MightBeFoldExpr && !Line->InMacroBody &&
           Style.RemoveParentheses > FormatStyle::RPS_Leave) {
         const auto *Next = Tokens->peekNextToken();
@@ -2565,7 +2566,6 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
         const bool CommaSeparated =
             !DoubleParens && Prev && Prev->isOneOf(tok::l_paren, tok::comma) &&
             Next && Next->isOneOf(tok::comma, tok::r_paren);
-        const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
         const bool Excluded =
             PrevPrev &&
             (PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
@@ -2585,9 +2585,14 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
           FormatTok->Optional = true;
         }
       }
-      if (Prev && Prev->is(TT_TypenameMacro)) {
-        LeftParen->setFinalizedType(TT_TypeDeclarationParen);
-        FormatTok->setFinalizedType(TT_TypeDeclarationParen);
+      if (Prev) {
+        if (Prev->is(TT_TypenameMacro)) {
+          LeftParen->setFinalizedType(TT_TypeDeclarationParen);
+          FormatTok->setFinalizedType(TT_TypeDeclarationParen);
+        } else if (Prev->is(tok::greater) && FormatTok->Previous == LeftParen &&
+                   PrevPrev && PrevPrev->isNot(tok::kw_operator)) {
+          Prev->setFinalizedType(TT_TemplateCloser);
+        }
       }
       nextToken();
       return SeenEqual;
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 1884d41a5f23f5..5264b4e1265fbc 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -3505,6 +3505,13 @@ TEST_F(TokenAnnotatorTest, SplitPenalty) {
   EXPECT_SPLIT_PENALTY(Tokens[7], 23u);
 }
 
+TEST_F(TokenAnnotatorTest, TemplateInstantiation) {
+  auto Tokens = annotate("return FixedInt<N | M>();");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_TemplateCloser);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

>From 203a61fab155398ad0b59bd41552d20367d9d3c8 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sun, 29 Sep 2024 21:19:42 -0700
Subject: [PATCH 2/2] No need to check if the token before the `>` is
 `operator`

---
 clang/lib/Format/UnwrappedLineParser.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 1f8013e3e4858f..a38a86764d68c5 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2557,7 +2557,6 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
       break;
     case tok::r_paren: {
       auto *Prev = LeftParen->Previous;
-      const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
       if (!MightBeStmtExpr && !MightBeFoldExpr && !Line->InMacroBody &&
           Style.RemoveParentheses > FormatStyle::RPS_Leave) {
         const auto *Next = Tokens->peekNextToken();
@@ -2566,6 +2565,7 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
         const bool CommaSeparated =
             !DoubleParens && Prev && Prev->isOneOf(tok::l_paren, tok::comma) &&
             Next && Next->isOneOf(tok::comma, tok::r_paren);
+        const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
         const bool Excluded =
             PrevPrev &&
             (PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
@@ -2589,8 +2589,7 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
         if (Prev->is(TT_TypenameMacro)) {
           LeftParen->setFinalizedType(TT_TypeDeclarationParen);
           FormatTok->setFinalizedType(TT_TypeDeclarationParen);
-        } else if (Prev->is(tok::greater) && FormatTok->Previous == LeftParen &&
-                   PrevPrev && PrevPrev->isNot(tok::kw_operator)) {
+        } else if (Prev->is(tok::greater) && FormatTok->Previous == LeftParen) {
           Prev->setFinalizedType(TT_TemplateCloser);
         }
       }



More information about the cfe-commits mailing list