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 <owenpi...@gmail.com> Date: Sun, 1 Oct 2023 23:01:30 -0700 Subject: [PATCH 1/2] [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) { >From 05281416496f09d85ce67e0d90cd98c2e2d22c57 Mon Sep 17 00:00:00 2001 From: Owen Pan <owenpi...@gmail.com> Date: Tue, 3 Oct 2023 13:42:40 -0700 Subject: [PATCH 2/2] [clang-format] Simplify a test case --- clang/unittests/Format/FormatTestMacroExpansion.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index 741271a96dad739..db6401cea9614fb 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), ; }); -)", - Style); + verifyIncompleteFormat("ID3({, ID(a *b), ; });", Style); verifyFormat("ID(CALL(CALL(return a * b;)));", Style); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits