[clang-format] Don't count template template parameter as declaration

[rymiel]

Reapply 4a7bf42
which was reverted in 34d44eb

Not sure why there are tests elsewhere in clang that rely on the output of clang-format, but they were wrong

[llvmbot]

@llvm/pr-subscribers-clang-format

@llvm/pr-subscribers-clang

Author: Emilia Kond (rymiel)

Changes

Reapply 4a7bf42
which was reverted in 34d44eb

Not sure why there are tests elsewhere in clang that rely on the output of clang-format, but they were wrong

---

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

3 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+19-11)
• (modified) clang/test/Index/overriding-ftemplate-comments.cpp (+2-2)
• (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+17)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 55c5ecee45e0c..89e134144d433 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -127,7 +127,7 @@ class AnnotatingParser {
                    SmallVector<ScopeType> &Scopes)
       : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
         IsCpp(Style.isCpp()), LangOpts(getFormattingLangOpts(Style)),
-        Keywords(Keywords), Scopes(Scopes) {
+        Keywords(Keywords), Scopes(Scopes), TemplateDeclarationDepth(0) {
     assert(IsCpp == LangOpts.CXXOperatorNames);
     Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
     resetTokenMetadata();
@@ -1266,16 +1266,22 @@ class AnnotatingParser {
   }
 
   bool parseTemplateDeclaration() {
-    if (CurrentToken && CurrentToken->is(tok::less)) {
-      CurrentToken->setType(TT_TemplateOpener);
-      next();
-      if (!parseAngle())
-        return false;
-      if (CurrentToken)
-        CurrentToken->Previous->ClosesTemplateDeclaration = true;
-      return true;
-    }
-    return false;
+    if (!CurrentToken || CurrentToken->isNot(tok::less))
+      return false;
+
+    CurrentToken->setType(TT_TemplateOpener);
+    next();
+
+    TemplateDeclarationDepth++;
+    const bool WellFormed = parseAngle();
+    TemplateDeclarationDepth--;
+    if (!WellFormed)
+      return false;
+
+    if (CurrentToken && TemplateDeclarationDepth == 0)
+      CurrentToken->Previous->ClosesTemplateDeclaration = true;
+
+    return true;
   }
 
   bool consumeToken() {
@@ -3091,6 +3097,8 @@ class AnnotatingParser {
   // same decision irrespective of the decisions for tokens leading up to it.
   // Store this information to prevent this from causing exponential runtime.
   llvm::SmallPtrSet<FormatToken *, 16> NonTemplateLess;
+
+  int TemplateDeclarationDepth;
 };
 
 static const int PrecedenceUnaryOperator = prec::PointerToMember + 1;
diff --git a/clang/test/Index/overriding-ftemplate-comments.cpp b/clang/test/Index/overriding-ftemplate-comments.cpp
index 169d45f288e63..370b4cb293a0a 100644
--- a/clang/test/Index/overriding-ftemplate-comments.cpp
+++ b/clang/test/Index/overriding-ftemplate-comments.cpp
@@ -77,10 +77,10 @@ void comment_to_html_conversion_21();
 template <class C1, template <class C2, template <class C3, class C4> class BBB > class AAA>
 void comment_to_html_conversion_22();
 
-// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#v#</USR><Declaration>template &lt;class C1, template &lt;class C2, template &lt;class C3, class C4&gt; class BBB&gt;\n                    class AAA&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>C1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>AAA</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>C2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>C3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>C4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>BBB</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
+// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#v#</USR><Declaration>template &lt;\n    class C1,\n    template &lt;class C2, template &lt;class C3, class C4&gt; class BBB&gt; class AAA&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>C1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>AAA</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>C2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>C3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>C4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>BBB</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
 
 template<class CCC1, template<class CCC2, template<class CCC3, class CCC4> class QQQ> class PPP>
 void comment_to_html_conversion_22();
 
-// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#v#</USR><Declaration>template &lt;class CCC1,\n          template &lt;class CCC2, template &lt;class CCC3, class CCC4&gt; class QQQ&gt;\n          class PPP&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>CCC1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>PPP</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>CCC2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>CCC3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>CCC4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>QQQ</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
+// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#v#</USR><Declaration>template &lt;class CCC1,\n          template &lt;class CCC2,\n                    template &lt;class CCC3, class CCC4&gt; class QQQ&gt; class PPP&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>CCC1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>PPP</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>CCC2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>CCC3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>CCC4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>QQQ</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
 
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 12c4b7fdd5ac2..d3b310fe59527 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -584,6 +584,23 @@ TEST_F(TokenAnnotatorTest, UnderstandsNonTemplateAngleBrackets) {
   EXPECT_TOKEN(Tokens[20], tok::greater, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTemplateTemplateParameters) {
+  auto Tokens = annotate("template <template <typename...> typename X,\n"
+                         "          template <typename...> class Y,\n"
+                         "          typename... T>\n"
+                         "class A {};");
+  ASSERT_EQ(Tokens.size(), 28u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[3], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::greater, TT_TemplateCloser);
+  EXPECT_FALSE(Tokens[6]->ClosesTemplateDeclaration);
+  EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[14], tok::greater, TT_TemplateCloser);
+  EXPECT_FALSE(Tokens[14]->ClosesTemplateDeclaration);
+  EXPECT_TOKEN(Tokens[21], tok::greater, TT_TemplateCloser);
+  EXPECT_TRUE(Tokens[21]->ClosesTemplateDeclaration);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");

[mydeveloperday]

Doesn't this show a change in default formatting from

template <class CCC1,
          template <class CCC2, template <class CCC3, class CCC4> class QQQ>
          class PPP>
void comment_to_html_conversion_22()

template <class CCC1,
          template <class CCC2,
                    template <class CCC3, class CCC4> class QQQ> class PPP>
void comment_to_html_conversion_22()

I think the fact that the previous test failed shows more that we have changed the default. Is this the change we expect? maybe we could add a unit test to assert the new behavior if that is desired

[rymiel]

Isn't fixing bugs meant to change defaults? See #61732, #48746 and #93793. The old version is confusing since it looks like a separate template parameter when it isn't.

[mydeveloperday]

Isn't fixing bugs meant to change defaults? See #61732, #48746 and #93793. The old version is confusing since it looks like a separate template parameter when it isn't.

I'm ok, if the second style is what we want if thats what we are expecting then I'm good with it..