jaredgrubb updated this revision to Diff 504486.
jaredgrubb added a comment.
- Fixed an issue in `TokenAnnotator` about it not breaking between macros
properly (it was catching in an ObjC selector-check too early)
- Add more ObjC tests, covering method and property declarations too. There are
still some quirks about reflowing multiple attributes, but those quirks exist
in C++ too, so I think those are best left for another patch. I added checks
for existing behavior so that patch can improve the ObjC version too.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145262/new/
https://reviews.llvm.org/D145262
Files:
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/ContinuationIndenter.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestObjC.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===================================================================
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1296,6 +1296,121 @@
EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
}
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacros) {
+ // '__attribute__' has special handling.
+ auto Tokens = annotate("__attribute__(X) void Foo(void);");
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+
+ // Generic macro has no special handling in this location.
+ Tokens = annotate("A(X) void Foo(void);");
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::identifier, TT_Unknown);
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+ // 'TT_FunctionAnnotationRParen' doesn't seem right; fix?
+ EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_FunctionAnnotationRParen);
+
+ // Add a custom AttributeMacro. Test that it has the same behavior.
+ FormatStyle Style = getLLVMStyle();
+ Style.AttributeMacros.push_back("A");
+
+ // An "AttributeMacro" gets annotated like '__attribute__'.
+ Tokens = annotate("A(X) void Foo(void);", Style);
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCDecl) {
+ // '__attribute__' has special handling.
+ auto Tokens = annotate("__attribute__(X) @interface Foo");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+
+ // Generic macro has no special handling in this location.
+ Tokens = annotate("A(X) @interface Foo");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ // Note: Don't check token-type as a random token in this position is hard to
+ // reason about.
+ EXPECT_TOKEN_KIND(Tokens[0], tok::identifier);
+ EXPECT_TOKEN_KIND(Tokens[1], tok::l_paren);
+ EXPECT_TOKEN_KIND(Tokens[3], tok::r_paren);
+
+ // Add a custom AttributeMacro. Test that it has the same behavior.
+ FormatStyle Style = getLLVMStyle();
+ Style.AttributeMacros.push_back("A");
+
+ // An "AttributeMacro" gets annotated like '__attribute__'.
+ Tokens = annotate("A(X) @interface Foo", Style);
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCMethodDecl) {
+ // '__attribute__' has special handling.
+ auto Tokens = annotate("- (id)init __attribute__(X);");
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[5], tok::kw___attribute, TT_Unknown);
+ EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeParen);
+
+ // Generic macro has no special handling in this location.
+ Tokens = annotate("- (id)init A(X);");
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ // Note: Don't check token-type as a random token in this position is hard to
+ // reason about.
+ EXPECT_TOKEN_KIND(Tokens[5], tok::identifier);
+ EXPECT_TOKEN_KIND(Tokens[6], tok::l_paren);
+ EXPECT_TOKEN_KIND(Tokens[8], tok::r_paren);
+
+ // Add a custom AttributeMacro. Test that it has the same behavior.
+ FormatStyle Style = getLLVMStyle();
+ Style.AttributeMacros.push_back("A");
+
+ // An "AttributeMacro" gets annotated like '__attribute__'.
+ Tokens = annotate("- (id)init A(X);", Style);
+ ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+ EXPECT_TOKEN(Tokens[5], tok::identifier, TT_AttributeMacro);
+ EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCProperty) {
+ // '__attribute__' has special handling.
+ auto Tokens = annotate("@property(weak) id delegate __attribute__(X);");
+ ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+ EXPECT_TOKEN(Tokens[7], tok::kw___attribute, TT_Unknown);
+ EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_AttributeParen);
+
+ // Generic macro has no special handling in this location.
+ Tokens = annotate("@property(weak) id delegate A(X);");
+ ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+ // Note: Don't check token-type as a random token in this position is hard to
+ // reason about.
+ EXPECT_TOKEN_KIND(Tokens[7], tok::identifier);
+ EXPECT_TOKEN_KIND(Tokens[8], tok::l_paren);
+ EXPECT_TOKEN_KIND(Tokens[10], tok::r_paren);
+
+ // Add a custom AttributeMacro. Test that it has the same behavior.
+ FormatStyle Style = getLLVMStyle();
+ Style.AttributeMacros.push_back("A");
+
+ // An "AttributeMacro" gets annotated like '__attribute__'.
+ Tokens = annotate("@property(weak) id delegate A(X);", Style);
+ ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+ EXPECT_TOKEN(Tokens[7], tok::identifier, TT_AttributeMacro);
+ EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_AttributeParen);
+ EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_AttributeParen);
+}
+
TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
auto Annotate = [this](llvm::StringRef Code) {
return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
Index: clang/unittests/Format/FormatTestObjC.cpp
===================================================================
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1488,7 +1488,10 @@
" [obj func:arg2];");
}
-TEST_F(FormatTestObjC, Attributes) {
+TEST_F(FormatTestObjC, AttributesOnObjCDecl) {
+ Style.AttributeMacros.push_back("ATTRIBUTE_MACRO");
+
+ // Check '__attribute__' macro directly.
verifyFormat("__attribute__((objc_subclassing_restricted))\n"
"@interface Foo\n"
"@end");
@@ -1498,6 +1501,211 @@
verifyFormat("__attribute__((objc_subclassing_restricted))\n"
"@implementation Foo\n"
"@end");
+
+ // Check AttributeMacro gets treated the same, with or without parentheses.
+ verifyFormat("ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+
+ // Indenter also needs to understand multiple attribute macros.
+ // Try each of the three kinds paired with each of the other kind.
+
+ // Column limit, but no reflow.
+ verifyFormat("ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X)) ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO __attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X)) ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO(X) __attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+
+ // Column limit that requires reflow.
+ Style.ColumnLimit = 30;
+ verifyFormat("ATTRIBUTE_MACRO(X)\n"
+ "ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO\n"
+ "ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X))\n"
+ "ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO\n"
+ "__attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X))\n"
+ "ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO(X)\n"
+ "__attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+
+ // No column limit
+ Style.ColumnLimit = 0;
+ verifyFormat("ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X)) ATTRIBUTE_MACRO\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO __attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("__attribute__((X)) ATTRIBUTE_MACRO(X)\n"
+ "@interface Foo\n"
+ "@end");
+ verifyFormat("ATTRIBUTE_MACRO(X) __attribute__((X))\n"
+ "@interface Foo\n"
+ "@end");
+}
+
+TEST_F(FormatTestObjC, AttributesOnObjCMethodDecl) {
+ Style.AttributeMacros.push_back("ATTRIBUTE_MACRO");
+
+ // Check '__attribute__' macro directly.
+ verifyFormat("- (id)init __attribute__((objc_designated_initializer));");
+
+ // Check AttributeMacro gets treated the same, with or without parentheses.
+ verifyFormat("- (id)init ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X);");
+
+ // Indenter also needs to understand multiple attribute macros.
+
+ // Column limit (default), but no reflow.
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init __attribute__((X)) ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO __attribute__((X));");
+ verifyFormat("- (id)init __attribute__((X)) ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X) __attribute__((X));");
+
+ // Column limit that requires reflow.
+ Style.ColumnLimit = 30;
+
+ // Reflow after method name.
+ verifyFormat("- (id)initWithReallyLongName\n"
+ " __attribute__((X))\n"
+ " ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)initWithReallyLongName\n"
+ " ATTRIBUTE_MACRO(X)\n"
+ " ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)initWithReallyLongName\n"
+ " ATTRIBUTE_MACRO\n"
+ " ATTRIBUTE_MACRO;");
+ // Reflow after first macro.
+ // FIXME: these should indent but don't.
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"
+ "ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO\n"
+ "ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init __attribute__((X))\n"
+ "ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO\n"
+ "__attribute__((X));");
+ verifyFormat("- (id)init __attribute__((X))\n"
+ "ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"
+ "__attribute__((X));");
+
+ // No column limit.
+ Style.ColumnLimit = 0;
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init __attribute__((X)) ATTRIBUTE_MACRO;");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO __attribute__((X));");
+ verifyFormat("- (id)init __attribute__((X)) ATTRIBUTE_MACRO(X);");
+ verifyFormat("- (id)init ATTRIBUTE_MACRO(X) __attribute__((X));");
+}
+
+TEST_F(FormatTestObjC, AttributesOnObjCProperty) {
+ Style.AttributeMacros.push_back("ATTRIBUTE_MACRO");
+
+ // Check '__attribute__' macro directly.
+ verifyFormat("@property(weak) id delegate "
+ "__attribute__((objc_designated_initializer));");
+
+ // Check AttributeMacro gets treated the same, with or without parentheses.
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO;");
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO(X);");
+
+ // Indenter also needs to understand multiple attribute macros.
+
+ // Column limit (default), but no reflow.
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO;");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X);");
+ verifyFormat(
+ "@property(weak) id delegate __attribute__((X)) ATTRIBUTE_MACRO;");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO __attribute__((X));");
+ verifyFormat(
+ "@property(weak) id delegate __attribute__((X)) ATTRIBUTE_MACRO(X);");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO(X) __attribute__((X));");
+
+ // Column limit that requires reflow.
+ Style.ColumnLimit = 50;
+
+ // Reflow after method name.
+ verifyFormat("@property(weak) id delegateWithLongName\n"
+ " __attribute__((X)) ATTRIBUTE_MACRO;");
+ verifyFormat("@property(weak) id delegateWithLongName\n"
+ " ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO;");
+ verifyFormat("@property(weak) id delegateWithLongName\n"
+ " ATTRIBUTE_MACRO ATTRIBUTE_MACRO;");
+ // Reflow after first macro.
+ // FIXME: these should indent but don't.
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO(X)\n"
+ "ATTRIBUTE_MACRO;");
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO\n"
+ "ATTRIBUTE_MACRO(X);");
+ verifyFormat("@property(weak) id delegate __attribute__((X))\n"
+ "ATTRIBUTE_MACRO;");
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO\n"
+ "__attribute__((X));");
+ verifyFormat("@property(weak) id delegate __attribute__((X))\n"
+ "ATTRIBUTE_MACRO(X);");
+ verifyFormat("@property(weak) id delegate ATTRIBUTE_MACRO(X)\n"
+ "__attribute__((X));");
+
+ // No column limit.
+ Style.ColumnLimit = 0;
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO(X) ATTRIBUTE_MACRO;");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO ATTRIBUTE_MACRO(X);");
+ verifyFormat(
+ "@property(weak) id delegate __attribute__((X)) ATTRIBUTE_MACRO;");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO __attribute__((X));");
+ verifyFormat(
+ "@property(weak) id delegate __attribute__((X)) ATTRIBUTE_MACRO(X);");
+ verifyFormat(
+ "@property(weak) id delegate ATTRIBUTE_MACRO(X) __attribute__((X));");
}
} // end namespace
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -369,7 +369,7 @@
// Infer the role of the l_paren based on the previous token if we haven't
// detected one yet.
if (PrevNonComment && OpeningParen.is(TT_Unknown)) {
- if (PrevNonComment->is(tok::kw___attribute)) {
+ if (PrevNonComment->isOneOf(tok::kw___attribute, TT_AttributeMacro)) {
OpeningParen.setType(TT_AttributeParen);
} else if (PrevNonComment->isOneOf(TT_TypenameMacro, tok::kw_decltype,
tok::kw_typeof,
@@ -4376,7 +4376,9 @@
if (Line.Type == LT_ObjCMethodDecl) {
if (Left.is(TT_ObjCMethodSpecifier))
return true;
- if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right)) {
+ // Apply this logic for parens that are not function attribute macros.
+ if ((Left.is(tok::r_paren) && !Left.is(TT_AttributeParen)) &&
+ canBeObjCSelectorComponent(Right)) {
// Don't space between ')' and <id> or ')' and 'new'. 'new' is not a
// keyword in Objective-C, and '+ (instancetype)new;' is a standard class
// method declaration.
@@ -4885,8 +4887,10 @@
}
// Ensure wrapping after __attribute__((XX)) and @interface etc.
- if (Left.is(TT_AttributeParen) && Right.is(TT_ObjCDecl))
+ if (Left.isOneOf(TT_AttributeMacro, TT_AttributeParen) &&
+ Right.is(TT_ObjCDecl)) {
return true;
+ }
if (Left.is(TT_LambdaLBrace)) {
if (IsFunctionArgument(Left) &&
@@ -5316,10 +5320,13 @@
tok::less, tok::coloncolon);
}
- if (Right.is(tok::kw___attribute) ||
- (Right.is(tok::l_square) && Right.is(TT_AttributeSquare))) {
+ // Breaking before a middle-of-line attribute macro is valid.
+ if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro))
+ return true;
+
+ // Don't split `[[` on C++ attributes.
+ if (Right.is(tok::l_square) && Right.is(TT_AttributeSquare))
return !Left.is(TT_AttributeSquare);
- }
if (Left.is(tok::identifier) && Right.is(tok::string_literal))
return true;
Index: clang/lib/Format/ContinuationIndenter.h
===================================================================
--- clang/lib/Format/ContinuationIndenter.h
+++ clang/lib/Format/ContinuationIndenter.h
@@ -228,8 +228,10 @@
/// The position of the last space on each level.
///
/// Used e.g. to break like:
+ /// ```
/// functionCall(Parameter, otherCall(
/// OtherParameter));
+ /// ```
unsigned LastSpace;
/// If a block relative to this parenthesis level gets wrapped, indent
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1228,8 +1228,9 @@
(PreviousNonComment->ClosesTemplateDeclaration ||
PreviousNonComment->ClosesRequiresClause ||
PreviousNonComment->isOneOf(
- TT_AttributeParen, TT_AttributeSquare, TT_FunctionAnnotationRParen,
- TT_JavaAnnotation, TT_LeadingJavaAnnotation))) ||
+ TT_AttributeParen, TT_AttributeMacro, TT_AttributeSquare,
+ TT_FunctionAnnotationRParen, TT_JavaAnnotation,
+ TT_LeadingJavaAnnotation))) ||
(!Style.IndentWrappedFunctionNames &&
NextNonComment->isOneOf(tok::kw_operator, TT_FunctionDeclarationName))) {
return std::max(CurrentState.LastSpace, CurrentState.Indent);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
