[clang] [clang-format] Annotate ctors/dtors as CtorDtorDeclName instead (PR #67955)
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 2 13:21:29 PDT 2023
https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/67955
>From 15d37075331311020020c5741e2432cd3fc0be74 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Sun, 1 Oct 2023 23:01:30 -0700
Subject: [PATCH] [clang-format] Annotate ctors/dtors as CtorDtorDeclName
instead
After annotating constructors/destructors as FunctionDeclarationName in
commit 08630512088, we have seen several issues because ctors/dtors had been
treated differently than functions in aligning, wrapping, and indenting.
This patch annotates ctors/dtors as CtorDtorDeclName instead and would
effectively revert commit 0468fa07f87f, which is obsolete now.
Fixed #67903.
Fixed #67907.
---
clang/lib/Format/FormatToken.h | 1 +
clang/lib/Format/TokenAnnotator.cpp | 18 ++++---
clang/lib/Format/WhitespaceManager.cpp | 6 +--
clang/unittests/Format/FormatTest.cpp | 10 +++-
.../Format/FormatTestMacroExpansion.cpp | 8 +--
clang/unittests/Format/TokenAnnotatorTest.cpp | 54 +++++++++++--------
6 files changed, 56 insertions(+), 41 deletions(-)
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index dbd3a6e70f037ef..5877b0a6124742a 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -61,6 +61,7 @@ namespace format {
TYPE(CSharpStringLiteral) \
TYPE(CtorInitializerColon) \
TYPE(CtorInitializerComma) \
+ TYPE(CtorDtorDeclName) \
TYPE(DesignatedInitializerLSquare) \
TYPE(DesignatedInitializerPeriod) \
TYPE(DictLiteral) \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 3ea65707da90369..e1c85d8a08fbf09 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3210,9 +3210,6 @@ static bool isCtorOrDtorName(const FormatToken *Tok) {
}
void TokenAnnotator::annotate(AnnotatedLine &Line) {
- for (auto &Child : Line.Children)
- annotate(*Child);
-
AnnotatingParser Parser(Style, Line, Keywords, Scopes);
Line.Type = Parser.parseLine();
@@ -3233,7 +3230,7 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) {
auto *Tok = getFunctionName(Line);
if (Tok && ((!Scopes.empty() && Scopes.back() == ST_Class) ||
Line.endsWith(TT_FunctionLBrace) || isCtorOrDtorName(Tok))) {
- Tok->setFinalizedType(TT_FunctionDeclarationName);
+ Tok->setFinalizedType(TT_CtorDtorDeclName);
}
}
@@ -3246,6 +3243,9 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) {
Line.First->SpacesRequiredBefore = 1;
Line.First->CanBreakBefore = Line.First->MustBreakBefore;
+
+ for (auto &Child : Line.Children)
+ annotate(*Child);
}
// This function heuristically determines whether 'Current' starts the name of a
@@ -3447,9 +3447,13 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
Tok = Tok->Next) {
if (Tok->Previous->EndsCppAttributeGroup)
AfterLastAttribute = Tok;
- if (isFunctionDeclarationName(Style.isCpp(), *Tok, Line, ClosingParen)) {
- LineIsFunctionDeclaration = true;
- Tok->setFinalizedType(TT_FunctionDeclarationName);
+ if (const bool IsCtorOrDtor = Tok->is(TT_CtorDtorDeclName);
+ IsCtorOrDtor ||
+ isFunctionDeclarationName(Style.isCpp(), *Tok, Line, ClosingParen)) {
+ if (!IsCtorOrDtor) {
+ LineIsFunctionDeclaration = true;
+ Tok->setFinalizedType(TT_FunctionDeclarationName);
+ }
if (AfterLastAttribute &&
mustBreakAfterAttributes(*AfterLastAttribute, Style)) {
AfterLastAttribute->MustBreakBefore = true;
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 762729d1c4166a5..1790a9df42b5d14 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -974,11 +974,7 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
AlignTokens(
Style,
[](Change const &C) {
- if (C.Tok->is(TT_FunctionDeclarationName) && C.Tok->Previous &&
- C.Tok->Previous->isNot(tok::tilde)) {
- return true;
- }
- if (C.Tok->is(TT_FunctionTypeLParen))
+ if (C.Tok->isOneOf(TT_FunctionDeclarationName, TT_FunctionTypeLParen))
return true;
if (C.Tok->isNot(TT_StartOfName))
return false;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 63ef294ce9d2949..a3723b421f161ef 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10622,6 +10622,12 @@ TEST_F(FormatTest, WrapsAtNestedNameSpecifiers) {
verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
" .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();");
+
+ verifyFormat(
+ "LongClassNameToShowTheIssue::AndAnotherLongClassNameToShowTheIssue::\n"
+ " AndAnotherLongClassNameToShowTheIssue() {}\n"
+ "LongClassNameToShowTheIssue::AndAnotherLongClassNameToShowTheIssue::\n"
+ " ~AndAnotherLongClassNameToShowTheIssue() {}");
}
TEST_F(FormatTest, UnderstandsTemplateParameters) {
@@ -16339,7 +16345,7 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
verifyFormat("int f();", SpaceFuncDef);
verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
- verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
+ verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
verifyFormat("#define A(x) x", SpaceFuncDef);
verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16364,7 +16370,7 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
// verifyFormat("T A::operator() () {}", SpaceFuncDef);
verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
verifyFormat("int x = int(y);", SpaceFuncDef);
- verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
+ verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
FormatStyle SpaceIfMacros = getLLVMStyle();
diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp
index 1ac5ac0d84f1251..741271a96dad739 100644
--- a/clang/unittests/Format/FormatTestMacroExpansion.cpp
+++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp
@@ -47,9 +47,7 @@ TEST_F(FormatTestMacroExpansion, UnexpandConfiguredMacros) {
{ ID(a *b); });
)",
Style);
- verifyIncompleteFormat(R"(ID3({, ID(a *b),
- ;
- });
+ verifyIncompleteFormat(R"(ID3({, ID(a *b), ; });
)",
Style);
@@ -251,9 +249,7 @@ TEST_F(FormatTestMacroExpansion,
ContinueFormattingAfterUnclosedParensAfterObjectLikeMacro) {
FormatStyle Style = getLLVMStyle();
Style.Macros.push_back("O=class {");
- verifyIncompleteFormat("O(auto x = [](){\n"
- " f();}",
- Style);
+ verifyIncompleteFormat("O(auto x = [](){f();}", Style);
}
} // namespace
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index a8259543dc488c0..62ec460eba7fd90 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1631,65 +1631,77 @@ TEST_F(TokenAnnotatorTest, UnderstandsFunctionDeclarationNames) {
ASSERT_EQ(Tokens.size(), 12u) << Tokens;
EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
- Tokens = annotate("class Foo { public: Foo(); };");
+ Tokens = annotate("#define FOO Foo::\n"
+ "FOO Foo();");
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+ Tokens = annotate("struct Foo {\n"
+ " Bar (*func)();\n"
+ "};");
+ ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+ EXPECT_TOKEN(Tokens[3], tok::identifier, TT_Unknown);
+ EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_FunctionTypeLParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCtorAndDtorDeclNames) {
+ auto Tokens = annotate("class Foo { public: Foo(); };");
ASSERT_EQ(Tokens.size(), 12u) << Tokens;
- EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[5], tok::identifier, TT_CtorDtorDeclName);
Tokens = annotate("class Foo { public: ~Foo(); };");
ASSERT_EQ(Tokens.size(), 13u) << Tokens;
- EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[6], tok::identifier, TT_CtorDtorDeclName);
Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
ASSERT_EQ(Tokens.size(), 16u) << Tokens;
- EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[8], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
ASSERT_EQ(Tokens.size(), 17u) << Tokens;
- EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[9], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
ASSERT_EQ(Tokens.size(), 16u) << Tokens;
- EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[3], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
ASSERT_EQ(Tokens.size(), 17u) << Tokens;
- EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[4], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
ASSERT_EQ(Tokens.size(), 17u) << Tokens;
- EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[9], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
ASSERT_EQ(Tokens.size(), 18u) << Tokens;
- EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[10], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("Foo::Foo() {}");
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
- EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[2], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
Tokens = annotate("Foo::~Foo() {}");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
- EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+ EXPECT_TOKEN(Tokens[3], tok::identifier, TT_CtorDtorDeclName);
EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
- Tokens = annotate("#define FOO Foo::\n"
- "FOO Foo();");
- ASSERT_EQ(Tokens.size(), 11u) << Tokens;
- EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
-
- Tokens = annotate("struct Foo {\n"
- " Bar (*func)();\n"
+ Tokens = annotate("struct Test {\n"
+ " Test()\n"
+ " : l([] {\n"
+ " Short::foo();\n"
+ " }) {}\n"
"};");
- ASSERT_EQ(Tokens.size(), 14u) << Tokens;
- EXPECT_TOKEN(Tokens[3], tok::identifier, TT_Unknown);
- EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_FunctionTypeLParen);
+ ASSERT_EQ(Tokens.size(), 25u) << Tokens;
+ EXPECT_TOKEN(Tokens[3], tok::identifier, TT_CtorDtorDeclName);
+ EXPECT_TOKEN(Tokens[14], tok::identifier, TT_Unknown);
}
TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
More information about the cfe-commits
mailing list