From 7590b7657004b33b1a12b05f8e9174db3e192f8c Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Mon, 28 Aug 2023 10:28:48 +0200 Subject: [PATCH] Revert "[clang-format] Annotate constructor/destructor names" This reverts commit 0e63f1aacc0040e9a16fa2fab15a140b1686e2ab. clang-format started to crash with contents like: a.h: ``` ``` $ clang-format a.h ``` PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. Program arguments: ../llvm/build/bin/clang-format a.h #0 0x0000560b689fe177 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Unix/Signals.inc:723:13 #1 0x0000560b689fbfbe llvm::sys::RunSignalHandlers() /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Signals.cpp:106:18 #2 0x0000560b689feaca SignalHandler(int) /usr/local/google/home/kadircet/repos/llvm/llvm/lib/Support/Unix/Signals.inc:413:1 #3 0x00007f030405a540 (/lib/x86_64-linux-gnu/libc.so.6+0x3c540) #4 0x0000560b68a9a980 is /usr/local/google/home/kadircet/repos/llvm/clang/include/clang/Lex/Token.h:98:44 #5 0x0000560b68a9a980 is /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:562:51 #6 0x0000560b68a9a980 startsSequenceInternal /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:831:9 #7 0x0000560b68a9a980 startsSequence /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:600:12 #8 0x0000560b68a9a980 getFunctionName /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/TokenAnnotator.cpp:3131:17 #9 0x0000560b68a9a980 clang::format::TokenAnnotator::annotate(clang::format::AnnotatedLine&) /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/TokenAnnotator.cpp:3191:17 Segmentation fault ``` --- clang/lib/Format/TokenAnnotator.cpp | 88 +-------------- clang/unittests/Format/FormatTest.cpp | 100 +++--------------- clang/unittests/Format/TokenAnnotatorTest.cpp | 48 --------- 3 files changed, 19 insertions(+), 217 deletions(-) diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 97df0b447b4baa..b6b4172818d171 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3097,76 +3097,6 @@ static unsigned maxNestingDepth(const AnnotatedLine &Line) { return Result; } -// Returns the name of a function with no return type, e.g. a constructor or -// destructor. -static FormatToken *getFunctionName(const AnnotatedLine &Line) { - for (FormatToken *Tok = Line.getFirstNonComment(), *Name = nullptr; Tok; - Tok = Tok->getNextNonComment()) { - // Skip C++11 attributes both before and after the function name. - if (Tok->is(tok::l_square) && Tok->is(TT_AttributeSquare)) { - Tok = Tok->MatchingParen; - if (!Tok) - break; - continue; - } - - // Make sure the name is followed by a pair of parentheses. - if (Name) - return Tok->is(tok::l_paren) && Tok->MatchingParen ? Name : nullptr; - - // Skip keywords that may precede the constructor/destructor name. - if (Tok->isOneOf(tok::kw_friend, tok::kw_inline, tok::kw_virtual, - tok::kw_constexpr, tok::kw_consteval, tok::kw_explicit)) { - continue; - } - - // A qualified name may start from the global namespace. - if (Tok->is(tok::coloncolon)) { - Tok = Tok->Next; - if (!Tok) - break; - } - - // Skip to the unqualified part of the name. - while (Tok->startsSequence(tok::identifier, tok::coloncolon)) { - assert(Tok->Next); - Tok = Tok->Next->Next; - if (!Tok) - break; - } - - // Skip the `~` if a destructor name. - if (Tok->is(tok::tilde)) { - Tok = Tok->Next; - if (!Tok) - break; - } - - // Make sure the name is not already annotated, e.g. as NamespaceMacro. - if (Tok->isNot(tok::identifier) || Tok->isNot(TT_Unknown)) - break; - - Name = Tok; - } - - return nullptr; -} - -// Checks if Tok is a constructor/destructor name qualified by its class name. -static bool isCtorOrDtorName(const FormatToken *Tok) { - assert(Tok && Tok->is(tok::identifier)); - const auto *Prev = Tok->Previous; - - if (Prev && Prev->is(tok::tilde)) - Prev = Prev->Previous; - - if (!Prev || !Prev->endsSequence(tok::coloncolon, tok::identifier)) - return false; - - assert(Prev->Previous); - return Prev->Previous->TokenText == Tok->TokenText; -} - void TokenAnnotator::annotate(AnnotatedLine &Line) { for (auto &Child : Line.Children) annotate(*Child); @@ -3187,14 +3117,6 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) { ExpressionParser ExprParser(Style, Keywords, Line); ExprParser.parse(); - if (Style.isCpp()) { - auto *Tok = getFunctionName(Line); - if (Tok && ((!Scopes.empty() && Scopes.back() == ST_Class) || - Line.endsWith(TT_FunctionLBrace) || isCtorOrDtorName(Tok))) { - Tok->setFinalizedType(TT_FunctionDeclarationName); - } - } - if (Line.startsWith(TT_ObjCMethodSpecifier)) Line.Type = LT_ObjCMethodDecl; else if (Line.startsWith(TT_ObjCDecl)) @@ -3211,10 +3133,6 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) { static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current, const AnnotatedLine &Line) { assert(Current.Previous); - - if (Current.is(TT_FunctionDeclarationName)) - return true; - if (!Current.Tok.getIdentifierInfo()) return false; @@ -3395,11 +3313,9 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const { bool LineIsFunctionDeclaration = false; for (FormatToken *Tok = Current, *AfterLastAttribute = nullptr; Tok; Tok = Tok->Next) { - if (Tok->Previous->EndsCppAttributeGroup) - AfterLastAttribute = Tok; if (isFunctionDeclarationName(Style.isCpp(), *Tok, Line)) { LineIsFunctionDeclaration = true; - Tok->setFinalizedType(TT_FunctionDeclarationName); + Tok->setType(TT_FunctionDeclarationName); if (AfterLastAttribute && mustBreakAfterAttributes(*AfterLastAttribute, Style)) { AfterLastAttribute->MustBreakBefore = true; @@ -3407,6 +3323,8 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const { } break; } + if (Tok->Previous->EndsCppAttributeGroup) + AfterLastAttribute = Tok; } if (Style.isCpp() && !LineIsFunctionDeclaration) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index a0a6b942d8dfa6..c1368c947af1ef 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -16541,7 +16541,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); @@ -16566,7 +16566,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(); @@ -26239,18 +26239,18 @@ TEST_F(FormatTest, BreakAfterAttributes) { FormatStyle Style = getLLVMStyle(); EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never); - constexpr StringRef Code("[[nodiscard]] inline int f(int &i);\n" - "[[foo([[]])]] [[nodiscard]]\n" - "int g(int &i);\n" - "[[nodiscard]]\n" - "inline int f(int &i) {\n" - " i = 1;\n" - " return 0;\n" - "}\n" - "[[foo([[]])]] [[nodiscard]] int g(int &i) {\n" - " i = 0;\n" - " return 1;\n" - "}"); + const StringRef Code("[[nodiscard]] inline int f(int &i);\n" + "[[foo([[]])]] [[nodiscard]]\n" + "int g(int &i);\n" + "[[nodiscard]]\n" + "inline int f(int &i) {\n" + " i = 1;\n" + " return 0;\n" + "}\n" + "[[foo([[]])]] [[nodiscard]] int g(int &i) {\n" + " i = 0;\n" + " return 1;\n" + "}"); verifyFormat("[[nodiscard]] inline int f(int &i);\n" "[[foo([[]])]] [[nodiscard]] int g(int &i);\n" @@ -26264,9 +26264,6 @@ TEST_F(FormatTest, BreakAfterAttributes) { "}", Code, Style); - Style.BreakAfterAttributes = FormatStyle::ABS_Leave; - verifyNoChange(Code, Style); - Style.BreakAfterAttributes = FormatStyle::ABS_Always; verifyFormat("[[nodiscard]]\n" "inline int f(int &i);\n" @@ -26284,73 +26281,8 @@ TEST_F(FormatTest, BreakAfterAttributes) { "}", Code, Style); - constexpr StringRef CtorDtorCode("struct Foo {\n" - " [[deprecated]] Foo();\n" - " [[deprecated]] Foo() {}\n" - " [[deprecated]] ~Foo();\n" - " [[deprecated]] ~Foo() {}\n" - " [[deprecated]] void f();\n" - " [[deprecated]] void f() {}\n" - "};\n" - "[[deprecated]] Bar::Bar() {}\n" - "[[deprecated]] Bar::~Bar() {}\n" - "[[deprecated]] void g() {}"); - verifyFormat("struct Foo {\n" - " [[deprecated]]\n" - " Foo();\n" - " [[deprecated]]\n" - " Foo() {}\n" - " [[deprecated]]\n" - " ~Foo();\n" - " [[deprecated]]\n" - " ~Foo() {}\n" - " [[deprecated]]\n" - " void f();\n" - " [[deprecated]]\n" - " void f() {}\n" - "};\n" - "[[deprecated]]\n" - "Bar::Bar() {}\n" - "[[deprecated]]\n" - "Bar::~Bar() {}\n" - "[[deprecated]]\n" - "void g() {}", - CtorDtorCode, Style); - - Style.BreakBeforeBraces = FormatStyle::BS_Linux; - verifyFormat("struct Foo {\n" - " [[deprecated]]\n" - " Foo();\n" - " [[deprecated]]\n" - " Foo()\n" - " {\n" - " }\n" - " [[deprecated]]\n" - " ~Foo();\n" - " [[deprecated]]\n" - " ~Foo()\n" - " {\n" - " }\n" - " [[deprecated]]\n" - " void f();\n" - " [[deprecated]]\n" - " void f()\n" - " {\n" - " }\n" - "};\n" - "[[deprecated]]\n" - "Bar::Bar()\n" - "{\n" - "}\n" - "[[deprecated]]\n" - "Bar::~Bar()\n" - "{\n" - "}\n" - "[[deprecated]]\n" - "void g()\n" - "{\n" - "}", - CtorDtorCode, Style); + Style.BreakAfterAttributes = FormatStyle::ABS_Leave; + verifyNoChange(Code, Style); } TEST_F(FormatTest, InsertNewlineAtEOF) { diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index a20a38ae070eb9..ae2084923de00e 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -1589,54 +1589,6 @@ TEST_F(TokenAnnotatorTest, UnderstandsFunctionDeclarationNames) { Tokens = annotate("void f [[noreturn]] () {}"); ASSERT_EQ(Tokens.size(), 12u) << Tokens; EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName); - - Tokens = annotate("class Foo { public: Foo(); };"); - ASSERT_EQ(Tokens.size(), 12u) << Tokens; - EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName); - - Tokens = annotate("class Foo { public: ~Foo(); };"); - ASSERT_EQ(Tokens.size(), 13u) << Tokens; - EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName); - - Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };"); - ASSERT_EQ(Tokens.size(), 16u) << Tokens; - EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName); - 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[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[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[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[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[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[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[6], tok::l_brace, TT_FunctionLBrace); } TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {