[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 14 14:29:15 PDT 2023


owenpan added inline comments.


================
Comment at: clang/lib/Format/ContinuationIndenter.h:231
   /// Used e.g. to break like:
+  /// \code
   /// functionCall(Parameter, otherCall(
----------------
Unrelated.


================
Comment at: clang/lib/Format/ContinuationIndenter.h:234
   ///                             OtherParameter));
+  /// \endcode
   unsigned LastSpace;
----------------
Ditto.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:5520-5525
+  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);
----------------
I'd delete the comment on line 5519.


================
Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619
+  // Reflow after first macro.
+  // FIXME: these should indent but don't.
+  verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"
----------------
jaredgrubb wrote:
> 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!)
Half of the test cases passed before this patch but now would fail with this patch. That is, this patch would generate regressions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145262/new/

https://reviews.llvm.org/D145262



More information about the cfe-commits mailing list