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] Fix a bug in annotating StartOfName #127545

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Feb 17, 2025

Also ensure we can break before ClassHeadName like StartOfName.

Fixes #127470

Also ensure we can break before ClassHeadName like StartOfName.

Fixes llvm#127470
@llvmbot
Copy link
Member

llvmbot commented Feb 17, 2025

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Also ensure we can break before ClassHeadName like StartOfName.

Fixes #127470


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

3 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+3-3)
  • (modified) clang/unittests/Format/FormatTest.cpp (+5)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+4)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 069fd40e2834c..e68daa422b7c4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2596,7 +2596,7 @@ class AnnotatingParser {
         (!NextNonComment && !Line.InMacroBody) ||
         (NextNonComment &&
          (NextNonComment->isPointerOrReference() ||
-          NextNonComment->is(tok::string_literal) ||
+          NextNonComment->isOneOf(TT_ClassHeadName, tok::string_literal) ||
           (Line.InPragmaDirective && NextNonComment->is(tok::identifier))))) {
       return false;
     }
@@ -6198,8 +6198,8 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
                 FormatStyle::PAS_Right &&
             (!Right.Next || Right.Next->isNot(TT_FunctionDeclarationName)));
   }
-  if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
-      Right.is(tok::kw_operator)) {
+  if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+                    TT_ClassHeadName, tok::kw_operator)) {
     return true;
   }
   if (Left.is(TT_PointerOrReference))
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 2365a7c40bf76..d6d028436d39c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -29028,6 +29028,11 @@ TEST_F(FormatTest, WrapNamespaceBodyWithEmptyLinesAlways) {
                Style);
 }
 
+TEST_F(FormatTest, BreakBeforeClassName) {
+  verifyFormat("class ABSL_ATTRIBUTE_TRIVIAL_ABI ABSL_NULLABILITY_COMPATIBLE\n"
+               "    ArenaSafeUniquePtr {};");
+}
+
 } // namespace
 } // namespace test
 } // namespace format
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 1d0870c818acc..8ada6c3daeaf6 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -3250,6 +3250,10 @@ TEST_F(TokenAnnotatorTest, StartOfName) {
   EXPECT_TOKEN(Tokens[0], tok::at, TT_ObjCDecl);
   EXPECT_TOKEN(Tokens[2], tok::identifier, TT_StartOfName);
 
+  Tokens = annotate("class FOO BAR C {};");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_Unknown); // Not StartOfName
+
   auto Style = getLLVMStyle();
   Style.StatementAttributeLikeMacros.push_back("emit");
   Tokens = annotate("emit foo = 0;", Style);

@owenca owenca merged commit 13de15c into llvm:main Feb 18, 2025
10 checks passed
@owenca owenca deleted the 127470 branch February 18, 2025 08:15
Copy link
Contributor

@mydeveloperday mydeveloperday left a comment

Choose a reason for hiding this comment

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

Looks good to me

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 18, 2025
Also ensure we can break before ClassHeadName like StartOfName.

Fixes llvm#127470

(cherry picked from commit 13de15c)
wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this pull request Feb 19, 2025
Also ensure we can break before ClassHeadName like StartOfName.

Fixes llvm#127470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang-format regression around class declarations with attributes
4 participants