Page MenuHomePhabricator

[clang-format] [PR45639] clang-format splits up the brackets of C++17 attribute [[ ]] when used with the first parameter
ClosedPublic

Authored by MyDeveloperDay on May 5 2020, 2:01 AM.

Details

Summary

https://bugs.llvm.org/show_bug.cgi?id=45639

clang-format incorrectly splits the [[ in a long argument list

void SomeLongClassName::ALongMethodNameInThatClass([[maybe_unused]] const shared_ptr<ALongTypeName>& argumentNameForThat
LongType) {

}

becomes

void SomeLongClassName::ALongMethodNameInThatClass([
    [maybe_unused]] const shared_ptr<ALongTypeName> &argumentNameForThatLongType) {

}

leaving one [ on the previous line

For a function with just 1 very long argument, clang-format chooses to split between the [[,

This revision adjusts the penalty between ( and [ to ensure this is the more likely break point

Diff Detail

Event Timeline

MyDeveloperDay created this revision.May 5 2020, 2:01 AM

I think we should never break apart the two [[ and ]] of attributes.
Running that example with -debug shows that we consider breaking between the two [ tokens a valid possibility.

  • C=1 means breaking before the token is OK, which is probably computed by TokenAnnotator::canBreakBefore:
% bin/clang-format -debug ~/test.cc
Line(0, FSC=0): void[T=85, OC=0] identifier[T=85, OC=5] coloncolon[T=85, OC=22] identifier[T=85, OC=24] l_paren[T=85, OC=50] l_square[T=85, OC=51] l_square[T=85, OC=52] identifier[T=85, OC=53] r_square[T=85, OC=65] r_square[T=85, OC=66] const[T=85, OC=68] identifier[T=85, OC=74] less[T=85, OC=84] identifier[T=85, OC=85] greater[T=85, OC=98] amp[T=85, OC=99] identifier[T=85, OC=101] identifier[T=85, OC=51] r_paren[T=85, OC=59] l_brace[T=21, OC=61] 
Line(0, FSC=0): r_brace[T=85, OC=0] 
Line(0, FSC=0): eof[T=85, OC=0] 
Run 0...
AnnotatedTokens(L=0):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=void L=4 PPK=2 FakeLParens= FakeRParens=0 II=0x219c910 Text='void'
 M=0 C=1 T=FunctionDeclarationName S=1 B=0 BK=0 P=80 Name=identifier L=22 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8210 Text='SomeLongClassName'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=coloncolon L=24 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=520 Name=identifier L=50 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8258 Text='ALongMethodNameInThatClass'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=51 PPK=1 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=AttributeSquare S=0 B=0 BK=0 P=140 Name=l_square L=52 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='['
 M=0 C=1 T=AttributeSquare S=0 B=0 BK=0 P=79 Name=l_square L=53 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='['
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=99 Name=identifier L=65 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8290 Text='maybe_unused'
 M=0 C=0 T=AttributeSquare S=0 B=0 BK=0 P=83 Name=r_square L=66 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=AttributeSquare S=0 B=0 BK=0 P=63 Name=r_square L=67 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=']'
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=43 Name=const L=73 PPK=2 FakeLParens= FakeRParens=0 II=0x219c3e8 Text='const'
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=43 Name=identifier L=84 PPK=2 FakeLParens= FakeRParens=0 II=0x21d82c8 Text='shared_ptr'
 M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=50 Name=less L=85 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='<'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=380 Name=identifier L=98 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8300 Text='ALongTypeName'
 M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=290 Name=greater L=99 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='>'
 M=0 C=1 T=PointerOrReference S=1 B=0 BK=0 P=230 Name=amp L=101 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='&'
 M=0 C=1 T=StartOfName S=0 B=0 BK=0 P=240 Name=identifier L=120 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8340 Text='argumentNameForThat'
 M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=150 Name=identifier L=129 PPK=2 FakeLParens= FakeRParens=0 II=0x21d8378 Text='LongType'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=r_paren L=130 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=FunctionLBrace S=1 B=0 BK=1 P=23 Name=l_brace L=132 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='{'
----

You could adapt that code to not break before the second TT_AttributeSquare in [[ or ]]; this will have the same effect and generalize.

You could adapt that code to not break before the second TT_AttributeSquare in [[ or ]]; this will have the same effect and generalize.

sounds good... let me try that change and see if we like it better

MyDeveloperDay planned changes to this revision.May 6 2020, 1:51 PM
MyDeveloperDay updated this revision to Diff 262725.EditedMay 7 2020, 12:11 PM

Simplify to ensure we don't break in between the two TT_AttributeSquares

krasimir accepted this revision.May 7 2020, 12:46 PM

Thank you!

This revision is now accepted and ready to land.May 7 2020, 12:46 PM
This revision was automatically updated to reflect the committed changes.