Skip to content

Commit

Permalink
[Format] Fix isStartOfName to recognize attributes (#76804)
Browse files Browse the repository at this point in the history
This addresses a problem with formatting attributes. Some context:
- eaff083 changed `isStartOfName` to fix problems inside
`#pragma`s, but this behavior changed formatting of attribute macros in
an undesirable way.
- efeb546 changed Google format style
to fix some widely used attributes.

Instead of changing the format style, this commit specializes behavior
introduced in eaff083 to `#pragma`s. This seems to work well in
both cases.

Also update the test with two `GUARDED_BY` directives. While the
formatting after efeb546 seems better,
this case is rare enough to not warrant the extra complexity. We are
reverting it back to the state it had before
efeb546.

---------

Co-authored-by: Owen Pan <owenpiano@gmail.com>
  • Loading branch information
ilya-biryukov and owenca committed Jan 15, 2024
1 parent dc01b59 commit 5723fce
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 9 deletions.
2 changes: 0 additions & 2 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,8 +1701,6 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
/*BasedOnStyle=*/"google",
},
};
GoogleStyle.AttributeMacros.push_back("GUARDED_BY");
GoogleStyle.AttributeMacros.push_back("ABSL_GUARDED_BY");

GoogleStyle.SpacesBeforeTrailingComments = 2;
GoogleStyle.Standard = FormatStyle::LS_Auto;
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2209,7 +2209,8 @@ class AnnotatingParser {
(!NextNonComment && !Line.InMacroBody) ||
(NextNonComment &&
(NextNonComment->isPointerOrReference() ||
NextNonComment->isOneOf(tok::identifier, tok::string_literal)))) {
NextNonComment->is(tok::string_literal) ||
(Line.InPragmaDirective && NextNonComment->is(tok::identifier))))) {
return false;
}

Expand Down
8 changes: 2 additions & 6 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//

#include "FormatTestBase.h"
#include "gmock/gmock.h"

#define DEBUG_TYPE "format-test"

Expand Down Expand Up @@ -8563,9 +8562,6 @@ TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
" __attribute__((unused));");

Style = getGoogleStyle();
ASSERT_THAT(Style.AttributeMacros,
testing::AllOf(testing::Contains("GUARDED_BY"),
testing::Contains("ABSL_GUARDED_BY")));

verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
Expand Down Expand Up @@ -10158,11 +10154,11 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
getGoogleStyleWithColumns(40));
verifyFormat("Tttttttttttttttttttttttt ppppppppppppppp\n"
" ABSL_GUARDED_BY(mutex1)\n"
" ABSL_GUARDED_BY(mutex2);",
" ABSL_GUARDED_BY(mutex2);",
getGoogleStyleWithColumns(40));
verifyFormat("Tttttt f(int a, int b)\n"
" ABSL_GUARDED_BY(mutex1)\n"
" ABSL_GUARDED_BY(mutex2);",
" ABSL_GUARDED_BY(mutex2);",
getGoogleStyleWithColumns(40));
// * typedefs
verifyGoogleFormat("typedef ATTR(X) char x;");
Expand Down

0 comments on commit 5723fce

Please sign in to comment.