-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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] Don't count template template parameter as declaration #95025
Conversation
In ContinuationIndenter::mustBreak, a break is required between a template declaration and the function/class declaration it applies to, if the template declaration spans multiple lines. However, this also includes template template parameters, which can cause extra erroneous line breaks in some declarations. This patch makes template template parameters not be counted as template declarations. Fixes llvm#93793
@llvm/pr-subscribers-clang-format Author: Emilia Kond (rymiel) ChangesIn ContinuationIndenter::mustBreak, a break is required between a template declaration and the function/class declaration it applies to, if the template declaration spans multiple lines. However, this also includes template template parameters, which can cause extra erroneous line breaks in some declarations. This patch makes template template parameters not be counted as template declarations. Fixes #93793 Full diff: https://github.com/llvm/llvm-project/pull/95025.diff 2 Files Affected:
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 1fe3b61a5a81f..9ed25d3e4c7ee 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();
@@ -1269,10 +1269,17 @@ class AnnotatingParser {
if (CurrentToken && CurrentToken->is(tok::less)) {
CurrentToken->setType(TT_TemplateOpener);
next();
- if (!parseAngle())
+ TemplateDeclarationDepth++;
+ if (!parseAngle()) {
+ TemplateDeclarationDepth--;
return false;
- if (CurrentToken)
+ }
+ TemplateDeclarationDepth--;
+ if (CurrentToken &&
+ !(TemplateDeclarationDepth > 0 &&
+ CurrentToken->isOneOf(tok::kw_typename, tok::kw_class))) {
CurrentToken->Previous->ClosesTemplateDeclaration = true;
+ }
return true;
}
return false;
@@ -3073,6 +3080,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/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 8cc5c239d30a1..82de72ddeeaa1 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...> typename 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_EQ(Tokens[6]->ClosesTemplateDeclaration, 0u);
+ EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
+ EXPECT_TOKEN(Tokens[14], tok::greater, TT_TemplateCloser);
+ EXPECT_EQ(Tokens[14]->ClosesTemplateDeclaration, 0u);
+ EXPECT_TOKEN(Tokens[21], tok::greater, TT_TemplateCloser);
+ EXPECT_EQ(Tokens[21]->ClosesTemplateDeclaration, 1u);
+}
+
TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
FormatStyle Style = getLLVMStyle();
Style.WhitespaceSensitiveMacros.push_back("FOO");
|
Note: I understand my solution of adding a member variable is inelegant, I am of course open to ways on making it better |
Co-authored-by: Björn Schäpers <github@hazardy.de>
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/636 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/577 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/12/builds/510 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/848 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/428 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/659 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/500 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/444 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/433 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/450 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/378 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/459 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/506 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/684 Here is the relevant piece of the build log for the reference:
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/164/builds/324 Here is the relevant piece of the build log for the reference:
|
…llvm#95025) In ContinuationIndenter::mustBreak, a break is required between a template declaration and the function/class declaration it applies to, if the template declaration spans multiple lines. However, this also includes template template parameters, which can cause extra erroneous line breaks in some declarations. This patch makes template template parameters not be counted as template declarations. Fixes llvm#93793 Fixes llvm#48746
…laration" (llvm#96388) Reverts llvm#95025 ; many bots are broken
In ContinuationIndenter::mustBreak, a break is required between a template declaration and the function/class declaration it applies to, if the template declaration spans multiple lines.
However, this also includes template template parameters, which can cause extra erroneous line breaks in some declarations.
This patch makes template template parameters not be counted as template declarations.
Fixes #93793
Fixes #48746