Skip to content

[clang-format] Handle AttributeMacro before access modifiers #95634

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

Merged
merged 5 commits into from
Jun 16, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Jun 15, 2024

Closes #95094.

@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Closes #95094.


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

4 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+6-1)
  • (modified) clang/lib/Format/TokenAnnotator.h (+1)
  • (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+17-18)
  • (modified) clang/unittests/Format/FormatTest.cpp (+15)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 1fe3b61a5a81f..ff00e772a75f4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1970,6 +1970,7 @@ class AnnotatingParser {
       }
     }
 
+    bool SeenAccessModifier = false;
     bool KeywordVirtualFound = false;
     bool ImportStatement = false;
 
@@ -1978,7 +1979,9 @@ class AnnotatingParser {
       ImportStatement = true;
 
     while (CurrentToken) {
-      if (CurrentToken->is(tok::kw_virtual))
+      if (CurrentToken->isAccessSpecifier())
+        SeenAccessModifier = true;
+      else if (CurrentToken->is(tok::kw_virtual))
         KeywordVirtualFound = true;
       if (Style.isJavaScript()) {
         // export {...} from '...';
@@ -1998,6 +2001,8 @@ class AnnotatingParser {
       if (!consumeToken())
         return LT_Invalid;
     }
+    if (SeenAccessModifier)
+      return LT_AccessModifier;
     if (KeywordVirtualFound)
       return LT_VirtualFunctionDecl;
     if (ImportStatement)
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index d19d3d061e40c..136880eca718b 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -22,6 +22,7 @@ namespace format {
 
 enum LineType {
   LT_Invalid,
+  LT_AccessModifier, // Contains public/protected/private followed by colon.
   LT_ImportStatement,
   LT_ObjCDecl, // An @interface, @implementation, or @protocol line.
   LT_ObjCMethodDecl,
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 4d53361aaf333..729f3d78f4a35 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -57,7 +57,7 @@ class LevelIndentTracker {
   /// Update the indent state given that \p Line is going to be formatted
   /// next.
   void nextLine(const AnnotatedLine &Line) {
-    Offset = getIndentOffset(*Line.First);
+    Offset = getIndentOffset(Line);
     // Update the indent level cache size so that we can rely on it
     // having the right size in adjustToUnmodifiedline.
     if (Line.Level >= IndentForLevel.size())
@@ -111,42 +111,41 @@ class LevelIndentTracker {
   ///
   /// For example, 'public:' labels in classes are offset by 1 or 2
   /// characters to the left from their level.
-  int getIndentOffset(const FormatToken &RootToken) {
+  int getIndentOffset(const AnnotatedLine &Line) {
     if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
         Style.isCSharp()) {
       return 0;
     }
 
-    auto IsAccessModifier = [this, &RootToken]() {
-      if (RootToken.isAccessSpecifier(Style.isCpp())) {
+    auto IsAccessModifier = [&](const FormatToken &RootToken) {
+      if (Line.Type == LT_AccessModifier || RootToken.isObjCAccessSpecifier())
         return true;
-      } else if (RootToken.isObjCAccessSpecifier()) {
-        return true;
-      }
+
+      const auto *Next = RootToken.Next;
+
       // Handle Qt signals.
-      else if (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
-               RootToken.Next && RootToken.Next->is(tok::colon)) {
-        return true;
-      } else if (RootToken.Next &&
-                 RootToken.Next->isOneOf(Keywords.kw_slots,
-                                         Keywords.kw_qslots) &&
-                 RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) {
+      if (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+          Next && Next->is(tok::colon)) {
         return true;
       }
-      // Handle malformed access specifier e.g. 'private' without trailing ':'.
-      else if (!RootToken.Next && RootToken.isAccessSpecifier(false)) {
+
+      if (Next && Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+          Next->Next && Next->Next->is(tok::colon)) {
         return true;
       }
-      return false;
+
+      // Handle malformed access specifier e.g. 'private' without trailing ':'.
+      return !Next && RootToken.isAccessSpecifier(false);
     };
 
-    if (IsAccessModifier()) {
+    if (IsAccessModifier(*Line.First)) {
       // The AccessModifierOffset may be overridden by IndentAccessModifiers,
       // in which case we take a negative value of the IndentWidth to simulate
       // the upper indent level.
       return Style.IndentAccessModifiers ? -Style.IndentWidth
                                          : Style.AccessModifierOffset;
     }
+
     return 0;
   }
 
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index fb57333858529..2ca85c7b70e65 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12912,6 +12912,15 @@ TEST_F(FormatTest, FormatsAccessModifiers) {
                "  int j;\n"
                "};",
                Style);
+  Style.AttributeMacros.push_back("FOO");
+  Style.AttributeMacros.push_back("BAR");
+  verifyFormat("struct foo {\n"
+               "FOO private:\n"
+               "  int i;\n"
+               "BAR private:\n"
+               "  int j;\n"
+               "};",
+               Style);
 
   FormatStyle NoEmptyLines = getLLVMStyle();
   NoEmptyLines.MaxEmptyLinesToKeep = 0;
@@ -26130,6 +26139,12 @@ TEST_F(FormatTest, IndentAccessModifiers) {
                "      int i;\n"
                "};",
                Style);
+  Style.AttributeMacros.push_back("FOO");
+  verifyFormat("class C {\n"
+               "   FOO public:\n"
+               "      int i;\n"
+               "};",
+               Style);
 }
 
 TEST_F(FormatTest, LimitlessStringsAndComments) {

@owenca owenca merged commit a106131 into llvm:main Jun 16, 2024
7 checks passed
@owenca owenca deleted the AccessModifier branch June 16, 2024 20:51
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] Considering macros as access modifiers
3 participants