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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to