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

Jared Grubb via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 7 22:47:03 PDT 2023


jaredgrubb added inline comments.


================
Comment at: clang/lib/Format/TokenAnnotator.cpp:5473
+  if (Right.isOneOf(tok::kw___attribute, TT_AttributeMacro))
+    return true;
+
----------------
HazardyKnusperkeks wrote:
> 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)`?
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));
```



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

https://reviews.llvm.org/D145262



More information about the cfe-commits mailing list