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] Handle template closer followed by empty paretheses #110408

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Sep 29, 2024

Fixes #109925.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 29, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #109925.


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

3 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+23-19)
  • (modified) clang/lib/Format/UnwrappedLineParser.cpp (+10-5)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+7)
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

@owenca owenca merged commit 14e1fef into llvm:main Oct 3, 2024
6 of 7 checks passed
@owenca owenca deleted the 109925 branch October 3, 2024 02:06
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] Regression in formatting of explicit NTTPs with bitwise operators.
3 participants