[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
owenca wrote:
Addressed in 0bac8f1729512a0b392908d6a57a00b0387a855f
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
bdunkin wrote:
Perhaps you are correct, however the comment was made on a different check
which _was_ removed. Since this PR has already been merged, I would invite you
to make your own PR to address this if you feel strongly enough about it.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
shafik wrote:
I don't see how `Tok` can ever be a `nullptr` here.
`Tok` is first set by
`FormatToken *Tok = Line.getFirstNonComment()`
If this could yield a `nullptr` than the first unchecked dereference would be
wrong.
The next time `Tok` is set is at:
`while (auto *Next = skipNameQualifier(Tok))`
Other assignments to `Tok` have explicit `nullptr` checks.
but this can't be a `nullptr` b/c the condition would be `false`
We have various continues and breaks but the for loop unconditionally checks
`Tok` so those can't yield a `nullptr` either.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
bdunkin wrote:
This comment was made on a previous commit that has since been force pushed
over. The check it mentioned was removed, and this comment is now pointing to a
different, necessary check. Without this check, the next line might try to call
a function on a nullptr.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
shafik wrote:
Static analysis flagged this code, contrary to comment, this code was not
removed.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
github-actions[bot] wrote: @bdunkin Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/owenca closed https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/owenca approved this pull request. https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
bdunkin wrote: My apologies for disappearing for a while on this change. I had to change focus at work for a while, but I am now able to pick this back up. I have updated the code to include all the suggested changes. https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
bdunkin wrote:
I have made this change.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
+return nullptr;
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(tok::coloncolon))
+return Tok->getNextNonComment();
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(TT_TemplateOpener)) {
+if (!Tok->MatchingParen)
+ return nullptr;
+
+Tok = Tok->MatchingParen;
+if (Tok->startsSequence(TT_TemplateCloser, tok::coloncolon))
+ return Tok->getNextNonComment()->getNextNonComment();
+ }
+
+ return nullptr;
bdunkin wrote:
This is a good simplification. I have made this change.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
bdunkin wrote:
I have made this change.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
bdunkin wrote:
I have removed this check. For those following along, the loop condition checks
for `Tok` being nullptr, and will exit, returning nullptr, so there is no need
to do it here.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
bdunkin wrote:
I've added this assert
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From bbbf42e25cd9cc93b188cf378e95938e5f6a4dc3 Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH 1/3] Fix identifiers not being marked as
constructor/destructor names if they are qualified by a template type, or
have template typename declarations in front of them.
---
clang/lib/Format/TokenAnnotator.cpp | 66 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 55
2 files changed, 114 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index 79cfa73001e54..972382cbfaa4b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3668,6 +3668,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
+return nullptr;
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(tok::coloncolon))
+return Tok->getNextNonComment();
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(TT_TemplateOpener)) {
+if (!Tok->MatchingParen)
+ return nullptr;
+
+Tok = Tok->MatchingParen;
+if (Tok->startsSequence(TT_TemplateCloser, tok::coloncolon))
+ return Tok->getNextNonComment()->getNextNonComment();
+ }
+
+ return nullptr;
+}
+
// Returns the name of a function with no return type, e.g. a constructor or
// destructor.
static FormatToken *getFunctionName(const AnnotatedLine &Line,
@@ -3697,6 +3727,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;
@@ -3705,12 +3750,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
// Skip the `~` if a destructor name.
if (Tok->is(tok::tilde)) {
@@ -3737,10 +3781,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::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 065d53f42f02b..5b1d3ceab25bc 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2455,6 +2455,61 @@ 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::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::~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 Foo::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(To
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
+return nullptr;
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(tok::coloncolon))
+return Tok->getNextNonComment();
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(TT_TemplateOpener)) {
+if (!Tok->MatchingParen)
+ return nullptr;
+
+Tok = Tok->MatchingParen;
+if (Tok->startsSequence(TT_TemplateCloser, tok::coloncolon))
+ return Tok->getNextNonComment()->getNextNonComment();
+ }
+
+ return nullptr;
owenca wrote:
```suggestion
if (Tok->is(TT_TemplateOpener)) {
Tok = Tok->MatchingParen;
if (!Tok)
return nullptr;
Tok = Tok->getNextNonComment();
if (!Tok)
return nullptr;
}
return Tok->is(tok::coloncolon) ? Tok->getNextNonComment() : nullptr;
```
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
owenca wrote:
```suggestion
while (auto *Next = skipNameQualifier(Tok))
```
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3669,12 +3718,11 @@ 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;
- if (!Tok)
-return nullptr;
-}
+while (FormatToken *Next = skipNameQualifier(Tok))
+ Tok = Next;
+
+if (!Tok)
+ return nullptr;
owenca wrote:
Redundant.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
owenca wrote:
```suggestion
if (!Tok)
```
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3632,6 +3632,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken *skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
owenca wrote:
```suggestion
assert(Tok);
// Qualified names must start with an identifier.
```
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
HazardyKnusperkeks wrote:
How do you build llvm?
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
bdunkin wrote:
I am on Windows, and neither of those commands appears to support Windows. Are
there Windows equivalents?
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
owenca wrote:
```suggestion
static FormatToken *skipNameQualifier(const FormatToken *Tok) {
```
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/owenca requested changes to this pull request. Please run `ninja clang-format-check-format` before `git push`. Also, run `ninja polly-check-format`, which gives the assertion failure below: ``` Assertion failed: (Tok->is(TT_TemplateOpener)), function getFunctionName, file TokenAnnotator.cpp, line 3699. ``` https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3651,6 +3674,21 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line, continue; } +// Skip past template typename declarations that may precede the +// constructor/destructor name bdunkin wrote: Done. https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 839d068df748189470f2c69eb47714425de9524b Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if they are qualified by a template type, or have template typename
declarations in front of them.
---
clang/lib/Format/TokenAnnotator.cpp | 65 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 55
2 files changed, 114 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index aed1672afac66..0a76b00cc6c5b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
+return nullptr;
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(tok::coloncolon))
+return Tok->getNextNonComment();
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(TT_TemplateOpener)) {
+if (!Tok->MatchingParen)
+ return nullptr;
+
+Tok = Tok->MatchingParen;
+if (Tok->startsSequence(TT_TemplateCloser, tok::coloncolon))
+ return Tok->getNextNonComment()->getNextNonComment();
+ }
+
+ return nullptr;
+}
+
// Returns the name of a function with no return type, e.g. a constructor or
// destructor.
static FormatToken *getFunctionName(const AnnotatedLine &Line,
@@ -3659,6 +3689,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;
@@ -3667,13 +3712,13 @@ 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;
- if (!Tok)
-return nullptr;
+while (FormatToken* Next = skipNameQualifier(Tok)) {
+ Tok = Next;
}
+if (!Tok)
+ return nullptr;
+
// Skip the `~` if a destructor name.
if (Tok->is(tok::tilde)) {
Tok = Tok->Next;
@@ -3699,10 +3744,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::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 873c6c492d18c..c3538b2a8edfa 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2351,6 +2351,61 @@ 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::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::~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 Foo::Foo() {}");
+ ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+ EXPECT_TOKEN(Tokens[10], tok::identifier, TT_CtorDtorDeclName);
+ EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_FunctionDeclarationLPare
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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)) {
bdunkin wrote:
I have done this, and renamed it `skipNameQualifier` to reflect the new
behaviour.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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::B::B()
+ // Tok --^
+ if (Tok->startsSequence(tok::identifier, TT_TemplateOpener)) {
+Tok = Tok->getNextNonComment();
+assert(Tok);
bdunkin wrote:
Done.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3622,6 +3622,29 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+static bool startsQualifiedName(const FormatToken *Tok) {
bdunkin wrote:
Done.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 37c7cde072e25eae0409e162c5080830d182f2dd Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if they are qualified by a template type, or have template typename
declarations in front of them.
---
clang/lib/Format/TokenAnnotator.cpp | 65 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 55
2 files changed, 114 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Format/TokenAnnotator.cpp
b/clang/lib/Format/TokenAnnotator.cpp
index aed1672afac66..307231b3b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3630,6 +3630,36 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+// Returns the token after the first qualifier of the name, or nullptr if there
+// is no qualifier.
+static FormatToken* skipNameQualifier(const FormatToken *Tok) {
+ // Qualified names must start with an identifier.
+ if (!Tok->is(tok::identifier))
+return nullptr;
+
+ Tok = Tok->getNextNonComment();
+ if (Tok == nullptr)
+return nullptr;
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(tok::coloncolon))
+return Tok->getNextNonComment();
+
+ // Consider: A::B::B()
+ //Tok --^
+ if (Tok->is(TT_TemplateOpener)) {
+if (!Tok->MatchingParen)
+ return nullptr;
+
+Tok = Tok->MatchingParen;
+if (Tok->startsSequence(TT_TemplateCloser, tok::coloncolon))
+ return Tok->getNextNonComment()->getNextNonComment();
+ }
+
+ return nullptr;
+}
+
// Returns the name of a function with no return type, e.g. a constructor or
// destructor.
static FormatToken *getFunctionName(const AnnotatedLine &Line,
@@ -3659,6 +3689,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;
@@ -3667,13 +3712,13 @@ 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;
- if (!Tok)
-return nullptr;
+while (FormatToken* Next = skipNameQualifier(Tok)) {
+ Tok = Next;
}
+if (!Tok)
+ return nullptr;
+
// Skip the `~` if a destructor name.
if (Tok->is(tok::tilde)) {
Tok = Tok->Next;
@@ -3699,10 +3744,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::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 873c6c492d18c..c3538b2a8edfa 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2351,6 +2351,61 @@ 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::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::~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 Foo::Foo() {}");
+ ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+ EXPECT_TOKEN(Tokens[10], tok::identifier, TT_CtorDtorDeclName);
+ EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_FunctionDeclarationLParen
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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)) {
HazardyKnusperkeks wrote:
My bad, I misread the indentation. But you could add a test case.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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)) {
bdunkin wrote:
This is already inside the loop. Do you perhaps mean that it should come
earlier?
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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)) {
HazardyKnusperkeks wrote:
Can you change the return value of `startsQualifiedName`, so that it returns
the `Tok` to continue and you don't need to recheck for the template?
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3651,6 +3674,21 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line, continue; } +// Skip past template typename declarations that may precede the +// constructor/destructor name HazardyKnusperkeks wrote: ```suggestion // Skip past template typename declarations that may precede the // constructor/destructor name. ``` Comments end in full stop. https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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::B::B()
+ // Tok --^
+ if (Tok->startsSequence(tok::identifier, TT_TemplateOpener)) {
+Tok = Tok->getNextNonComment();
+assert(Tok);
HazardyKnusperkeks wrote:
I think you can drop the asserts.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -3622,6 +3622,29 @@ static unsigned maxNestingDepth(const AnnotatedLine
&Line) {
return Result;
}
+static bool startsQualifiedName(const FormatToken *Tok) {
HazardyKnusperkeks wrote:
Id check for `tok::identifier` and then move to `nextNonComment()` for checking
`coloncolon` rsp. `TT_TemplateOpener`.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
@@ -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)) {
HazardyKnusperkeks wrote:
Shouldn't this be moved in the loop above? Consider `template
[[nodiscard]] Foo::Foo() {}`.
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/bdunkin updated
https://github.com/llvm/llvm-project/pull/143194
>From 0168891771b3cdfc6f6305b046005fb335cbff01 Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if they are qualified by a template type, or have template typename
declarations in front of them.
---
clang/lib/Format/TokenAnnotator.cpp | 68 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 43
2 files changed, 107 insertions(+), 4 deletions(-)
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::B::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::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..98a570a42ada3 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2346,6 +2346,49 @@ 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::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::~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 Foo::Foo() {}");
+ ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+ EXPECT_TOKEN(Tokens[10], tok::identifier, TT_CtorDtorDeclName);
+ EXPECT_TOKEN(Tokens[11], tok::l_paren,
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
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
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 7e9b9c24f..98a570a42 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2382,7 +2382,8 @@ TEST_F(TokenAnnotatorTest,
UnderstandsCtorAndDtorDeclNames) {
EXPECT_TOKEN(Tokens[17], tok::l_paren, TT_FunctionDeclarationLParen);
EXPECT_TOKEN(Tokens[19], tok::l_brace, TT_FunctionLBrace);
- Tokens = annotate("template template Foo::Foo(W
x) {}");
+ Tokens = annotate(
+ "template template Foo::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);
``
https://github.com/llvm/llvm-project/pull/143194
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
github-actions[bot] wrote: 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](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/143194 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
llvmbot wrote:
@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
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::B::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::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::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::~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 Foo::Foo() {}");
+ ASSERT_EQ(Tokens.size(), 16u) << To
[clang] [clang-format] Handle templates in qualified typenames (PR #143194)
https://github.com/bdunkin created
https://github.com/llvm/llvm-project/pull/143194
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
>From 2f262ca8d5060bf58ac23241917bad6d7347c8d3 Mon Sep 17 00:00:00 2001
From: Ben Dunkin
Date: Fri, 6 Jun 2025 12:29:13 -0700
Subject: [PATCH] Fix identifiers not being marked as constructor/destructor
names if they are qualified by a template type, or have template typename
declarations in front of them.
---
clang/lib/Format/TokenAnnotator.cpp | 68 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 42
2 files changed, 106 insertions(+), 4 deletions(-)
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::B::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::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::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::~Foo() {}");
+ ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+ EXPECT_TOKEN(Tokens
