curdeius accepted this revision.
curdeius added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:215
+ assert(CurrentToken->Previous && "Unknown previous token");
+ FormatToken &OpeningParen = *CurrentToken->Previous;
+ assert(OpeningParen.is(tok::l_paren));
----------------
Maybe you can commit the rename as a NFC commit and rebase the patch to remove
the noise?
================
Comment at: clang/unittests/Format/FormatTest.cpp:23894-23896
+ " }())::value &&\n"
+ " requires(T t) { t.bar(); } && "
+ "sizeof(T) <= 8;",
----------------
HazardyKnusperkeks wrote:
> I know we don't like changing tests, but I think there was a bug before which
> were never hit in our tests but here.
>
> Before
> ```
> M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2
> FakeLParens=5/ FakeRParens=0 II=0xf136c40 Text='decltype'
> M=0 C=0 T=Unknown S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='('
> M=0 C=1 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2
> FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text=']'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='('
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
> M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='->'
> M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2
> FakeLParens= FakeRParens=0 II=0xf130c60 Text='std'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
> M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2
> FakeLParens= FakeRParens=0 II=0xf130c88 Text='true_type'
> M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
> M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='}'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='('
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2
> FakeLParens= FakeRParens=1 II=0x0 Text=')'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text=')'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
> M=0 C=1 T=TrailingAnnotation S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90
> PPK=2 FakeLParens= FakeRParens=0 II=0xf130ca8 Text='value'
> M=0 C=1 T=BinaryOperator S=1 F=0 B=0 BK=0 P=25 Name=ampamp L=93 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='&&'
> ```
> after
> ```
> M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=130 Name=decltype L=42 PPK=2
> FakeLParens=5/ FakeRParens=0 II=0xee66c40 Text='decltype'
> M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=1 BK=0 P=23 Name=l_paren L=43 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='('
> M=0 C=0 T=LambdaLSquare S=0 F=0 B=0 BK=0 P=140 Name=l_square L=44 PPK=2
> FakeLParens=1/ FakeRParens=0 II=0x0 Text='['
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=79 Name=r_square L=45 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text=']'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=46 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='('
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=47 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
> M=0 C=1 T=LambdaArrow S=1 F=0 B=0 BK=0 P=150 Name=arrow L=50 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='->'
> M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=43 Name=identifier L=54 PPK=2
> FakeLParens= FakeRParens=0 II=0xee60c60 Text='std'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=coloncolon L=56 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
> M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=540 Name=identifier L=65 PPK=2
> FakeLParens= FakeRParens=0 II=0xee60c88 Text='true_type'
> M=0 C=0 T=LambdaLBrace S=1 F=0 B=0 BK=1 P=43 Name=l_brace L=67 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='{'
> M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=61 Name=r_brace L=80 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='}'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=l_paren L=81 PPK=2 FakeLParens=
> FakeRParens=0 II=0x0 Text='('
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=160 Name=r_paren L=82 PPK=2
> FakeLParens= FakeRParens=1 II=0x0 Text=')'
> M=0 C=0 T=TypeDeclarationParen S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=83 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text=')'
> M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon L=85 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='::'
> M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520 Name=identifier L=90 PPK=2
> FakeLParens= FakeRParens=0 II=0xee60ca8 Text='value'
> M=0 C=0 T=BinaryOperator S=1 F=0 B=0 BK=0 P=25 Name=ampamp L=93 PPK=2
> FakeLParens= FakeRParens=0 II=0x0 Text='&&'
> ```
>
> Because of the missing check it just reset all paren with a brace in it, in
> this case the `TypeDeclarationParen`. which results in a miss labeling of the
> `value` as an annotation.
Looks legit to me.
================
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:335
+
+ // Invalid Code, but we don't want to crash. See http://llvm.org/PR54350.
+ Tokens = annotate("bool r10 = requires (struct new_struct { int x; } s) { "
----------------
I'd prefer a valid C++ code (if it's not too complicated) if possible but
that's acceptable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121550/new/
https://reviews.llvm.org/D121550
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
