[clang] 7590b76 - Revert "[clang-format] Annotate constructor/destructor names"

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 28 01:32:20 PDT 2023


Author: Kadir Cetinkaya
Date: 2023-08-28T10:31:07+02:00
New Revision: 7590b7657004b33b1a12b05f8e9174db3e192f8c

URL: https://github.com/llvm/llvm-project/commit/7590b7657004b33b1a12b05f8e9174db3e192f8c
DIFF: https://github.com/llvm/llvm-project/commit/7590b7657004b33b1a12b05f8e9174db3e192f8c.diff

LOG: 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<clang::tok::TokenKind, clang::tok::TokenKind> /usr/local/google/home/kadircet/repos/llvm/clang/lib/Format/FormatToken.h:831:9
 #7 0x0000560b68a9a980 startsSequence<clang::tok::TokenKind, clang::tok::TokenKind> /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
```

Added: 
    

Modified: 
    clang/lib/Format/TokenAnnotator.cpp
    clang/unittests/Format/FormatTest.cpp
    clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 
    


################################################################################
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) {


        


More information about the cfe-commits mailing list