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