Skip to content
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] Correctly annotate braces of empty functions #72733

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Nov 18, 2023

Also fixed some existing test cases.

Fixed #57305.
Fixed #58251.

Also fixed some existing test cases.

Fixed llvm#57305.
Fixed llvm#58251.
@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2023

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Also fixed some existing test cases.

Fixed #57305.
Fixed #58251.


Full diff: https://github.com/llvm/llvm-project/pull/72733.diff

3 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+13)
  • (modified) clang/unittests/Format/FormatTest.cpp (+19-19)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+54)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 1d376cd8b5794dd..fed5ed88743ffb9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3477,6 +3477,19 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
     }
   }
 
+  if (IsCpp && LineIsFunctionDeclaration &&
+      Line.endsWith(tok::semi, tok::r_brace)) {
+    auto *Tok = Line.Last->Previous;
+    while (Tok->isNot(tok::r_brace))
+      Tok = Tok->Previous;
+    if (auto *LBrace = Tok->MatchingParen; LBrace) {
+      assert(LBrace->is(tok::l_brace));
+      Tok->setBlockKind(BK_Block);
+      LBrace->setBlockKind(BK_Block);
+      LBrace->setFinalizedType(TT_FunctionLBrace);
+    }
+  }
+
   if (IsCpp && SeenName && AfterLastAttribute &&
       mustBreakAfterAttributes(*AfterLastAttribute, Style)) {
     AfterLastAttribute->MustBreakBefore = true;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index b5021f924a80904..10b991c6926879c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -3415,7 +3415,7 @@ TEST_F(FormatTest, UnderstandsAccessSpecifiers) {
   verifyFormat("myFunc(public);");
   verifyFormat("std::vector<int> testVec = {private};");
   verifyFormat("private.p = 1;");
-  verifyFormat("void function(private...){};");
+  verifyFormat("void function(private...) {};");
   verifyFormat("if (private && public)");
   verifyFormat("private &= true;");
   verifyFormat("int x = private * public;");
@@ -16093,16 +16093,16 @@ TEST_F(FormatTest, ZeroTabWidth) {
   Tab.IndentWidth = 8;
   Tab.UseTab = FormatStyle::UT_Never;
   Tab.TabWidth = 0;
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t// line starts with '\t'\n"
                "};",
                Tab);
 
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t\t// line starts with '\t'\n"
@@ -16110,16 +16110,16 @@ TEST_F(FormatTest, ZeroTabWidth) {
                Tab);
 
   Tab.UseTab = FormatStyle::UT_ForIndentation;
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t// line starts with '\t'\n"
                "};",
                Tab);
 
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t\t// line starts with '\t'\n"
@@ -16127,16 +16127,16 @@ TEST_F(FormatTest, ZeroTabWidth) {
                Tab);
 
   Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t// line starts with '\t'\n"
                "};",
                Tab);
 
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t\t// line starts with '\t'\n"
@@ -16144,16 +16144,16 @@ TEST_F(FormatTest, ZeroTabWidth) {
                Tab);
 
   Tab.UseTab = FormatStyle::UT_AlignWithSpaces;
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t// line starts with '\t'\n"
                "};",
                Tab);
 
-  verifyFormat("void a(){\n"
-               "    // line starts with '\t'\n"
+  verifyFormat("void a() {\n"
+               "        // line starts with '\t'\n"
                "};",
                "void a(){\n"
                "\t\t// line starts with '\t'\n"
@@ -16161,7 +16161,7 @@ TEST_F(FormatTest, ZeroTabWidth) {
                Tab);
 
   Tab.UseTab = FormatStyle::UT_Always;
-  verifyFormat("void a(){\n"
+  verifyFormat("void a() {\n"
                "// line starts with '\t'\n"
                "};",
                "void a(){\n"
@@ -16169,7 +16169,7 @@ TEST_F(FormatTest, ZeroTabWidth) {
                "};",
                Tab);
 
-  verifyFormat("void a(){\n"
+  verifyFormat("void a() {\n"
                "// line starts with '\t'\n"
                "};",
                "void a(){\n"
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index a986806b7a44069..013dee8b51f1c5d 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2404,6 +2404,60 @@ TEST_F(TokenAnnotatorTest, NotStartOfName) {
   EXPECT_TOKEN(Tokens[4], tok::identifier, TT_Unknown);
 }
 
+TEST_F(TokenAnnotatorTest, BraceKind) {
+  auto Tokens = annotate("void f() {};");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[4], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[5], BK_Block);
+
+  Tokens = annotate("void f() override {};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[5], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[6], BK_Block);
+
+  Tokens = annotate("void f() noexcept(false) {};");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[8], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[9], BK_Block);
+
+  Tokens = annotate("auto f() -> void {};");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[6], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[7], BK_Block);
+
+  Tokens = annotate("void f() { /**/ };");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[4], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[6], BK_Block);
+
+  Tokens = annotate("void f() { //\n"
+                    "};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[4], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[6], BK_Block);
+
+  Tokens = annotate("void f() {\n"
+                    "  //\n"
+                    "};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_FunctionLBrace);
+  EXPECT_BRACE_KIND(Tokens[4], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[6], BK_Block);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang

Copy link
Member

@rymiel rymiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow!

@owenca owenca merged commit edad025 into llvm:main Nov 19, 2023
@owenca owenca deleted the 57305 branch November 19, 2023 23:10
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants