[clang-format] Handle templates in qualified typenames

[bdunkin]

This fixes the SpaceBeforeParensOptions.AfterFunctionDeclarationName and SpaceBeforeParensOptions.AfterFunctionDefinitionName options not adding spaces when a template type's constructor or destructor is forward declared or defined outside of the type definition.

Attribution Note - I have been authorized to contribute this change on behalf of my company: ArenaNet LLC

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Ben Dunkin (bdunkin)

Changes

This fixes the SpaceBeforeParensOptions.AfterFunctionDeclarationName and SpaceBeforeParensOptions.AfterFunctionDefinitionName options not adding spaces when a template type's constructor or destructor is forward declared or defined outside of the type definition.

Attribution Note - I have been authorized to contribute this change on behalf of my company: ArenaNet LLC

---

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

2 Files Affected:

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

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 37ab40ca97bff..479a8743c5a65 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3622,6 +3622,29 @@ static unsigned maxNestingDepth(const AnnotatedLine &Line) {
   return Result;
 }
 
+static bool startsQualifiedName(const FormatToken *Tok) {
+  // Consider:       A::B::B()
+  //           Tok --^
+  if (Tok->startsSequence(tok::identifier, tok::coloncolon))
+    return true;
+
+  // Consider:       A<float>::B<int>::B()
+  //           Tok --^
+  if (Tok->startsSequence(tok::identifier, TT_TemplateOpener)) {
+    Tok = Tok->getNextNonComment();
+    assert(Tok);
+    assert(Tok->is(TT_TemplateOpener));
+
+    if (!Tok->MatchingParen)
+      return false;
+
+    return Tok->MatchingParen->startsSequence(TT_TemplateCloser,
+                                              tok::coloncolon);
+  }
+
+  return false;
+}
+
 // Returns the name of a function with no return type, e.g. a constructor or
 // destructor.
 static FormatToken *getFunctionName(const AnnotatedLine &Line,
@@ -3651,6 +3674,21 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line,
       continue;
     }
 
+    // Skip past template typename declarations that may precede the
+    // constructor/destructor name
+    if (Tok->is(tok::kw_template)) {
+      Tok = Tok->getNextNonComment();
+      if (!Tok)
+        return nullptr;
+
+      assert(Tok->is(TT_TemplateOpener));
+      Tok = Tok->MatchingParen;
+      if (!Tok)
+        return nullptr;
+
+      continue;
+    }
+
     // A qualified name may start from the global namespace.
     if (Tok->is(tok::coloncolon)) {
       Tok = Tok->Next;
@@ -3659,9 +3697,23 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line,
     }
 
     // Skip to the unqualified part of the name.
-    while (Tok->startsSequence(tok::identifier, tok::coloncolon)) {
-      assert(Tok->Next);
-      Tok = Tok->Next->Next;
+    while (startsQualifiedName(Tok)) {
+      Tok = Tok->getNextNonComment();
+      if (!Tok)
+        return nullptr;
+
+      // Skip template types if this is a templated type name
+      if (Tok->is(TT_TemplateOpener)) {
+        Tok = Tok->MatchingParen;
+        if (!Tok)
+          return nullptr;
+
+        Tok = Tok->getNextNonComment();
+        if (!Tok)
+          return nullptr;
+      }
+
+      Tok = Tok->getNextNonComment();
       if (!Tok)
         return nullptr;
     }
@@ -3691,10 +3743,18 @@ static bool isCtorOrDtorName(const FormatToken *Tok) {
   if (Prev && Prev->is(tok::tilde))
     Prev = Prev->Previous;
 
-  if (!Prev || !Prev->endsSequence(tok::coloncolon, tok::identifier))
+  // Consider: A::A() and A<int>::A()
+  if (!Prev || (!Prev->endsSequence(tok::coloncolon, tok::identifier) &&
+                !Prev->endsSequence(tok::coloncolon, TT_TemplateCloser))) {
     return false;
+  }
 
   assert(Prev->Previous);
+  if (Prev->Previous->is(TT_TemplateCloser) && Prev->Previous->MatchingParen) {
+    Prev = Prev->Previous->MatchingParen;
+    assert(Prev->Previous);
+  }
+
   return Prev->Previous->TokenText == Tok->TokenText;
 }
 
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 9d62ff8d39a77..7e9b9c24f6a3a 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2346,6 +2346,48 @@ TEST_F(TokenAnnotatorTest, UnderstandsCtorAndDtorDeclNames) {
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_FunctionDeclarationLParen);
   EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
 
+  Tokens = annotate("Foo<int>::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo<int>::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("template <typename V> Foo<V>::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("template <typename V> Foo<V>::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[12], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("template <typename V, typename U> Foo<V, U>::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[16], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[18], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("template <typename V, typename U> Foo<V, U>::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[16], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[17], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[19], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("template <typename V> template<typename W> Foo<V>::Foo(W x) {}");
+  ASSERT_EQ(Tokens.size(), 23u) << Tokens;
+  EXPECT_TOKEN(Tokens[15], tok::identifier, TT_CtorDtorDeclName);
+  EXPECT_TOKEN(Tokens[16], tok::l_paren, TT_FunctionDeclarationLParen);
+  EXPECT_TOKEN(Tokens[20], tok::l_brace, TT_FunctionLBrace);
+
   Tokens = annotate("struct Test {\n"
                     "  Test()\n"
                     "      : l([] {\n"

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/TokenAnnotatorTest.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 0a76b00cc..909bdc556 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3632,7 +3632,7 @@ static unsigned maxNestingDepth(const AnnotatedLine &Line) {
 
 // Returns the token after the first qualifier of the name, or nullptr if there
 // is no qualifier.
-static FormatToken* skipNameQualifier(const FormatToken *Tok) {
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
   // Qualified names must start with an identifier.
   if (!Tok->is(tok::identifier))
     return nullptr;
@@ -3712,9 +3712,8 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line,
     }
 
     // Skip to the unqualified part of the name.
-    while (FormatToken* Next = skipNameQualifier(Tok)) {
+    while (FormatToken *Next = skipNameQualifier(Tok))
       Tok = Next;
-    }
 
     if (!Tok)
       return nullptr;

[HazardyKnusperkeks]

LGTM, let's see what @owenca has to say.

[HazardyKnusperkeks]

Suggested change

	if (!Tok->is(tok::identifier))
	if (Tok->isNot(tok::identifier))