[clang-format] Treat AttributeMacros more like __attribute__

[jaredgrubb]

Summary

I noticed that clang-format was inserting some strange indentation whenever I used custom "attribute-like macros"
(things like FOO_EXTERN to wrap attribute-visible-default, or macros with parentheses like NS_SWIFT_NAME(...)).

There are two parts to this fix:

• Annotate the paren after an AttributeMacro as an AttributeLParen.
• Treat an AttributeMacro-without-paren the same as one with a paren.

I added a new test-case to differentiate a macro that is or is-not an AttributeMacro; also handled whether ColumnLimit is set to infinite (0) or a finite value, as part of this patch is in ContinuationIndenter.

There may be other places that need to handle TT_AttributeMacro better, but this is at least a step in the right
direction.

Closes https://github.com/llvm/llvm-project/issues/68722.

[HazardyKnusperkeks]

Our tests reside in clang/unittests/Format, you want to look into the TokenAnnotatorTests. In fact I can't read the tests you wrote there.

And could you please provide an example what you want to format and where the misformatting (or apparently misannotation) happens?

[jaredgrubb]

For background, the current clang-format results in the following (-style="{BasedOnStyle: LLVM, ColumnLimit: 0, AttributeMacros: [MACRO]}):

MACRO MACRO(A)
    @interface Foo
@end

MACRO(A)
MACRO
    @interface Foo
@end

This patch improves it (removes the indention and makes both cases have same wrapping):

MACRO MACRO(A)
@interface Foo
@end

MACRO(A) MACRO
@interface Foo
@end

[jaredgrubb]

I wasn't sure about testing (this is my first patch!) and the test-case I did was in clang/test/Format .. I can look at clang/unittests/Format and see how to model something like it.

If I do that, would that be in-addition-to or instead-of the test-case I included already?

[owenpan]

In D145262#4168058, @jaredgrubb wrote:

I wasn't sure about testing (this is my first patch!) and the test-case I did was in clang/test/Format .. I can look at clang/unittests/Format and see how to model something like it.

If I do that, would that be in-addition-to or instead-of the test-case I included already?

Instead of.

[jaredgrubb]

Create unit-tests for the patch (and remove the proposed non-unit test "test").

[HazardyKnusperkeks]

clang/unittests/Format/FormatTestObjC.cpp

1533

So there is most likely a CanBreak not true? That happens also in the TokenAnnotator.

clang/unittests/Format/TokenAnnotatorTest.cpp

1506

Your bug is in regard to Objective-C, right? Maybe add a test for that here too?

[jaredgrubb]

• 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.

[jaredgrubb]

clang/lib/Format/TokenAnnotator.cpp

5477–5478

I broke this if into two pieces and restored an original return true for this part of the if that was changed in 2020 (via 5a4ddbd69db2b0e09398214510501d0e59a0c30b).

The return !Left... was added to avoid breaking [[ apart when reflowing a function-arg with C++11-style attribute (eg, [[unused]]). As far as I can tell, this check _really_ only matters if the code-path is the second-half of the original if (Right.is(tok::l_square)...).

I couldn't imagine how there Right.is(tok::kw___attribute) needs the Left-check, so I think that patch is better split to differentiate (even though originally it didn't really have any conflicting effect).

Please let me know if my logic here is wrong (or I'm not making sense :) )

[jaredgrubb]

clang/unittests/Format/FormatTestObjC.cpp

1624

I don't love this FIXME, but I was afraid to add more to this patch, as fixing this will require digging into things that have nothing to do with __attribute__ vs AttributeMacros.

For example, suffix macros in C/C++ also are broken in the same way with just plain __attribute__. For example, for ColumnWidth: 50:

int f(double) __attribute__((overloadable))
__attribute__((overloadable));

int ffffffffffffffffffffffffffffff(double)
    __attribute__((overloadable))
    __attribute__((overloadable));

I think fixing reflowing of suffix macros is best done in another PR (which I can take a stab at!)

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

4514

clang/unittests/Format/FormatTestObjC.cpp

1687–1698

clang/unittests/Format/TokenAnnotatorTest.cpp

1488–1494

I would delete it.

1516–1523

Ditto.

1545–1552

Ditto.

1574–1581

Ditto.

[jaredgrubb]

Address review comments from @owenpan

[jaredgrubb]

I have uploaded patch for all the points you called out. (Sorry for delay; I missed the suggestions earlier!)

[MyDeveloperDay]

clang/lib/Format/ContinuationIndenter.h

231

If you want this to look like code you need to use \code \endcode

[MyDeveloperDay]

clang/lib/Format/ContinuationIndenter.h

231

Ok my bad this is the ContinuationIndenter.h, not Format.h

[jaredgrubb]

Address review feedback about code/endcode. Otherwise the patch is the same and I hope ready for merge? I'd love to get a green check :)

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

4514–4515

5473

Does changing this return value make no difference? In other words is there no combination of Left.is(TT_AttributeSquare) and Right.is(tok::kw___attribute)?

[jaredgrubb]

clang/lib/Format/TokenAnnotator.cpp

5473

Yes, that combo can happen; example below.

The patch that changed the left-diff line from true to !Left.is(TT_AttributeSquare)
was done _only_ contemplating the [[ case (5a4ddbd69db2b0e09398214510501d0e59a0c30b); tagging @MyDeveloperDay who wrote that patch and can perhaps offer more insight on your question.

My reasoning is to revert that part of the patch just a bit and limit it to that case only.

I couldn't come up with a use-case where you'd want to avoid splitting between TT_AttributeSquare and kw___attribute, but the example below shows that allowing it to break in that combination is preferable to the alternative of breaking in the parens of the attribute:

// Style: "{BasedOnStyle: LLVM, ColumnLimit: 40}"
// Existing Behavior
int ffffffffffffffffffffffffffffff(
    double)
    __attribute__((overloadable))
    [[unused]] __attribute__((
        overloadable));

// With Patch
int ffffffffffffffffffffffffffffff(
    double)
    __attribute__((overloadable))
    [[unused]]
    __attribute__((overloadable));

[HazardyKnusperkeks]

clang/lib/Format/TokenAnnotator.cpp

5473

Thanks for the detailed answer. I'll wait for @MyDeveloperDay. You are right, it's prettier with the patch, but on the other hand it is not desirable to change the formatting (apart from fixing bugs) between versions.

[jaredgrubb]

clang/lib/Format/TokenAnnotator.cpp

5473

This part of the patch isn't necessary so let me revert it so it doesn't slow this patch down.

But, the unit tests don't even fail when I revert it -- which means I really should treat this part as its own change with a unit test that matters.

[jaredgrubb]

Address review comments:

• remove redundant &&
• remove part of patch that was not tested by any test and should be its own patch on its own merits

[jaredgrubb]

Rebased and added a line to Release Notes, no other changes. Have requested this to be merged if all builds are green (since this patch has been approved).

[owenpan]

clang/lib/Format/ContinuationIndenter.h

231

Unrelated.

234

Ditto.

clang/lib/Format/TokenAnnotator.cpp

5472–5477

I'd delete the comment on line 5519.

clang/unittests/Format/FormatTestObjC.cpp

1624

Half of the test cases passed before this patch but now would fail with this patch. That is, this patch would generate regressions.

[jaredgrubb]

Hopefully this will satisfy and we can merge! I didn't notice the Phabricator timeline and I'm hoping we can finish this before Oct 1.

Rebased.
Removed extraneous doc comment not related to patch (as requested).
Removed comment (as requested).
Added a #if 0 around some cases that should be fixed one day (as requested prior but I missed one of the chunks).

[jaredgrubb]

clang/unittests/Format/FormatTestObjC.cpp

1624

Just saw this comment. Yes, 3 of these did pass, but lots more in this file do NOT pass. I understand the desire to not "regress", but this patch improves so many other examples (as documented in these test cases). I can pick some of the worst if it helps, but otherwise, I'm not sure what I can do to address your comment.

[owenpan]

This looks OK to me. Please rebase after https://github.com/llvm/llvm-project/pull/67518 is merged.

In D145262#4650300, @jaredgrubb wrote:

Hopefully this will satisfy and we can merge! I didn't notice the Phabricator timeline and I'm hoping we can finish this before Oct 1.

The new deadline is Nov 15 according to https://discourse.llvm.org/t/update-on-github-pull-requests/71540/125.

clang/docs/ReleaseNotes.rst

474–475 ↗

(On Diff #557274)

Can you delete this as release notes are usually for new options?

clang/lib/Format/TokenAnnotator.cpp

5472–5474

clang/unittests/Format/FormatTestObjC.cpp

1624

I'm okay with fixing them in another patch.

[owenpan]

In D145262#4651483, @owenpan wrote:

This looks OK to me. Please rebase after https://github.com/llvm/llvm-project/pull/67518 is merged.

I have merged the PR. Commits cef9f40cd4fe and 151d0a4db321 should make this patch less brittle.

[jaredgrubb]

Address review comments, and adjusting the patch to address other merges since this patch was started.

[owenpan]

clang/lib/Format/ContinuationIndenter.cpp

1237–1241

Basically, insert the first two lines and undo the changes to the last three lines.

clang/lib/Format/TokenAnnotator.cpp

4197–4198

Please also add a test case for this.

4513–4515

Please delete this comment, which IMO doesn't quite match the code below.

[owenpan]

Can you add a github issue to show the behavior before this patch?

[jaredgrubb]

Addressed review feedback from Oct2 (@owenpan).
A new issue on Github was opened for this enhancement (https://github.com/llvm/llvm-project/issues/68722)

[owenpan]

LGTM

[owenpan]

@jaredgrubb please provide your authorship for git commit --author so that we can land the patch on your behalf.

[jaredgrubb]

In D145262#4653670, @owenpan wrote:

@jaredgrubb please provide your authorship for git commit --author so that we can land the patch on your behalf.

The best to use would be:

Jared Grubb <[e-mail]>

Thanks so much for your help on this!!

clang/lib/Format/ContinuationIndenter.h

struct ParenState {
 
   /// The position to which a specific parenthesis level needs to be
   /// indented.
   unsigned Indent;
 
   /// The position of the last space on each level.
   ///
   /// Used e.g. to break like:
+  /// \code
   /// functionCall(Parameter, otherCall(
   ///                             OtherParameter));
+  /// \endcode
   unsigned LastSpace;
 
   /// If a block relative to this parenthesis level gets wrapped, indent
   /// it this much.
   unsigned NestedBlockIndent;
 
   /// The position the first "<<" operator encountered on each level.
   ///

clang/lib/Format/ContinuationIndenter.cpp

unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   }
   if (NextNonComment->isOneOf(TT_CtorInitializerColon, TT_InheritanceColon,
                               TT_InheritanceComma)) {
     return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
   }
   if ((PreviousNonComment &&
        (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);
   }
   if (NextNonComment->is(TT_SelectorName)) {
     if (!CurrentState.ObjCSelectorNameFound) {
       unsigned MinIndent = CurrentState.Indent;
       if (shouldIndentWrappedSelectorName(Style, State.Line->Type)) {
         MinIndent = std::max(MinIndent,

clang/lib/Format/TokenAnnotator.cpp

if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
           OpeningParen.Previous &&
           OpeningParen.Previous->isOneOf(tok::kw_for, tok::kw_catch);
       Contexts.back().IsExpression = !IsForOrCatch;
     }
 
     // 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,
 #define TRANSFORM_TYPE_TRAIT_DEF(_, Trait) tok::kw___##Trait,
 #include "clang/Basic/TransformTypeTraits.def"
                                          tok::kw__Atomic)) {
         OpeningParen.setType(TT_TypeDeclarationParen);
         // decltype() and typeof() usually contain expressions.
if (Left.is(TT_ModulePartitionColon) &&
         Right.isOneOf(tok::identifier, tok::kw_private)) {
       return false;
     }
     if (Left.is(tok::ellipsis) && Right.is(tok::identifier) &&
         Line.First->is(Keywords.kw_import)) {
       return false;
     }
     // Space in __attribute__((attr)) ::type.
     if (Left.is(TT_AttributeParen) && Right.is(tok::coloncolon))
       return true;
 
     if (Left.is(tok::kw_operator))
       return Right.is(tok::coloncolon);
     if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
         !Left.opensScope() && Style.SpaceBeforeCpp11BracedList) {
       return true;
     }
     if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
if ((Left.is(tok::l_brace) &&
       return false;
     }
   }
   if (Left.is(TT_ImplicitStringLiteral))
     return Right.hasWhitespaceBefore();
   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.isNot(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.
       return false;
     }
   }
   if (Line.Type == LT_ObjCProperty &&
       (Right.is(tok::equal) || Left.is(tok::equal))) {
bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
   }
 
   if (Left.is(TT_ObjCBlockLBrace) &&
       Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never) {
     return true;
   }
 
   // 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) &&
         Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline) {
       return false;
     }
 
     if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
 
   // Allow breaking after a trailing annotation, e.g. after a method
   // declaration.
   if (Left.is(TT_TrailingAnnotation)) {
     return !Right.isOneOf(tok::l_brace, tok::semi, tok::equal, tok::l_paren,
                           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 !Left.is(TT_AttributeSquare);
+
+  // 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;
 
   if (Right.is(tok::identifier) && Right.Next && Right.Next->is(TT_DictLiteral))
     return true;
 
   if (Left.is(TT_CtorInitializerColon)) {
     return Style.BreakConstructorInitializers == FormatStyle::BCIS_AfterColon &&

clang/unittests/Format/FormatTestObjC.cpp

verifyFormat("if (argc < 5)\n"
                "  [obj func:arg2];");
 
   verifyFormat("if (argc < 5) [[unlikely]]\n"
                "  [obj func:arg];\n"
                "else [[likely]]\n"
                "  [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");
   verifyFormat("__attribute__((objc_subclassing_restricted))\n"
                "@protocol Foo\n"
                "@end");
   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.
+#if 0
+  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));");
+#endif
+
+  // 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
 } // namespace test
 } // end namespace format
 } // end namespace clang

clang/unittests/Format/TokenAnnotatorTest.cpp

 TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
   auto Tokens = annotate("auto f() -> int;");
   ASSERT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("auto operator->() -> int;");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
   EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("auto operator++(int) -> int;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("auto operator=() -> int;");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("auto operator=(int) -> int;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("auto foo() -> auto { return Val; }");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
 
   Tokens = annotate("struct S { auto bar() const -> int; };");
   ASSERT_EQ(Tokens.size(), 14u) << Tokens;
   EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
 
   // Not trailing return arrows
   Tokens = annotate("auto a = b->c;");
   ASSERT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
 
   Tokens = annotate("auto a = (b)->c;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
 
   Tokens = annotate("auto a = b()->c;");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
 
   Tokens = annotate("auto a = b->c();");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
 
   Tokens = annotate("decltype(auto) a = b()->c;");
   ASSERT_EQ(Tokens.size(), 13u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
 
   Tokens = annotate("void f() { auto a = b->c(); }");
   ASSERT_EQ(Tokens.size(), 16u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
 
   Tokens = annotate("void f() { auto a = b()->c; }");
   ASSERT_EQ(Tokens.size(), 16u) << Tokens;
   EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
 
   // Mixed
   Tokens = annotate("auto f() -> int { auto a = b()->c; }");
   ASSERT_EQ(Tokens.size(), 18u) << Tokens;
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
   EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
 }
 
+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);
+
+  // 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);
+
+  // 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);
+
+  // 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);
+
+  // 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));
   };
   // Test that unary operators get labeled as such and that operators like '++'
   // don't get split.
   tok::TokenKind Unary[] = {tok::plus,  tok::minus,    tok::exclaim,
                             tok::tilde, tok::amp,      tok::pipe,