MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: owenpan, hans, krasimir, mitchell-stellar,
sammccall, pawels.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay edited the summary of this revision.
Fix to address https://bugs.llvm.org/show_bug.cgi?id=45218
The `<>` in the following code means the `i < bits` and `v >` in the
following code gets incorrectly determined as a TemplateOpener/Closer (see
debug output below)
While this became more prevalent after D66332: [clang-format] Fix the bug that
joins template closer and > or >> <https://reviews.llvm.org/D66332> this isn't
the root cause, but instead as called out by PR45218 it's more to do with
`parseAngle()` and how it labels `<` and `>`
`
for (unsigned i = 0; i < nbits; ++i, bIt.next(), v = v >> 1)
`
This can be seen by the following debug output for the line, where we can see
that the <> have the wrong `T` value.
The tests pass and various directories within LLVM which are currently already
clang-formatted don't show any new errors (although that far from exhaustive
testing)
M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=for L=3 PPK=2 FakeLParens=
FakeRParens=0 II=0x1e0bf1b7c80 Text='for'
M=0 C=0 T=Unknown S=1 B=0 BK=0 P=23 Name=l_paren L=5 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='('
M=0 C=1 T=Unknown S=0 B=0 BK=0 P=1040 Name=unsigned L=13 PPK=2
FakeLParens=2/0/ FakeRParens=0 II=0x1e0bf1b7fc0 Text='unsigned'
M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=240 Name=identifier L=15 PPK=2
FakeLParens= FakeRParens=0 II=0x1e0bf1f12a0 Text='i'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=42 Name=equal L=17 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='='
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=44 Name=numeric_constant L=19 PPK=2
FakeLParens= FakeRParens=1 II=0x0 Text='0'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=semi L=20 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=';'
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=40 Name=identifier L=22 PPK=2
FakeLParens=10/ FakeRParens=0 II=0x1e0bf1f12a0 Text='i'
M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=50 Name=less L=23 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=28 PPK=2
FakeLParens=0/ FakeRParens=0 II=0x1e0bf1f12d0 Text='nbits'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=283 Name=semi L=29 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=';'
M=0 C=1 T=UnaryOperator S=1 B=0 BK=0 P=280 Name=plusplus L=32 PPK=2
FakeLParens=0/1/ FakeRParens=0 II=0x0 Text='++'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=340 Name=identifier L=33 PPK=2 FakeLParens=
FakeRParens=1 II=0x1e0bf1f12a0 Text='i'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=281 Name=comma L=34 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=','
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=281 Name=identifier L=38 PPK=2
FakeLParens=0/ FakeRParens=0 II=0x1e0bf1f1300 Text='bIt'
M=0 C=1 T=Unknown S=0 B=0 BK=0 P=430 Name=period L=39 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='.'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=283 Name=identifier L=43 PPK=2 FakeLParens=
FakeRParens=0 II=0x1e0bf1f1330 Text='next'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=283 Name=l_paren L=44 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='('
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=319 Name=r_paren L=45 PPK=2 FakeLParens=
FakeRParens=1 II=0x0 Text=')'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=281 Name=comma L=46 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=','
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=281 Name=identifier L=48 PPK=2
FakeLParens=2/ FakeRParens=0 II=0x1e0bf1f1360 Text='v'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=282 Name=equal L=50 PPK=2
FakeLParens= FakeRParens=0 II=0x0 Text='='
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=284 Name=identifier L=52 PPK=2 FakeLParens=
FakeRParens=3 II=0x1e0bf1f1360 Text='v'
M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=290 Name=greater L=53 PPK=2
FakeLParens= FakeRParens=0 II=0x0 Text='>'
M=0 C=0 T=BinaryOperator S=0 B=0 BK=0 P=50 Name=greater L=54 PPK=2
FakeLParens= FakeRParens=0 II=0x0 Text='>'
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=50 Name=numeric_constant L=56 PPK=2
FakeLParens= FakeRParens=2 II=0x0 Text='1'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=r_paren L=57 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=')'
This fix tries to avoid that issue by determining that a ";" between the `<`
and `>` would not be a template (I couldn't think of an example where that
would be the case, but I'm sure there are..
Afterwards the token are interpreted correctly for this line. (
M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=for L=3 PPK=2 FakeLParens=
FakeRParens=0 II=0x1c891ac41b0 Text='for'
M=0 C=0 T=Unknown S=1 B=0 BK=0 P=23 Name=l_paren L=5 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='('
M=0 C=1 T=Unknown S=0 B=0 BK=0 P=1040 Name=unsigned L=13 PPK=2
FakeLParens=2/0/ FakeRParens=0 II=0x1c891ac44f0 Text='unsigned'
M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=240 Name=identifier L=15 PPK=2
FakeLParens= FakeRParens=0 II=0x1c891abce10 Text='i'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=42 Name=equal L=17 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='='
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=44 Name=numeric_constant L=19 PPK=2
FakeLParens= FakeRParens=1 II=0x0 Text='0'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=semi L=20 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=';'
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=40 Name=identifier L=22 PPK=2
FakeLParens=10/ FakeRParens=0 II=0x1c891abce10 Text='i'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=50 Name=less L=24 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='<'
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=50 Name=identifier L=30 PPK=2 FakeLParens=
FakeRParens=1 II=0x1c891abce40 Text='nbits'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=semi L=31 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=';'
M=0 C=1 T=UnaryOperator S=1 B=0 BK=0 P=40 Name=plusplus L=34 PPK=2
FakeLParens=0/1/ FakeRParens=0 II=0x0 Text='++'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=100 Name=identifier L=35 PPK=2 FakeLParens=
FakeRParens=1 II=0x1c891abce10 Text='i'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=41 Name=comma L=36 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=','
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=41 Name=identifier L=40 PPK=2 FakeLParens=0/
FakeRParens=0 II=0x1c891abce70 Text='bIt'
M=0 C=1 T=Unknown S=0 B=0 BK=0 P=190 Name=period L=41 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='.'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=identifier L=45 PPK=2 FakeLParens=
FakeRParens=0 II=0x1c891abcea0 Text='next'
M=0 C=0 T=Unknown S=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 B=0 BK=0 P=79 Name=r_paren L=47 PPK=2 FakeLParens=
FakeRParens=1 II=0x0 Text=')'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=41 Name=comma L=48 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=','
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=41 Name=identifier L=50 PPK=2 FakeLParens=2/
FakeRParens=0 II=0x1c891abced0 Text='v'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=42 Name=equal L=52 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text='='
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=44 Name=identifier L=54 PPK=2
FakeLParens=10/ FakeRParens=0 II=0x1c891abced0 Text='v'
M=0 C=0 T=BinaryOperator S=1 B=0 BK=0 P=50 Name=greater L=56 PPK=2
FakeLParens= FakeRParens=0 II=0x0 Text='>'
M=0 C=0 T=BinaryOperator S=0 B=0 BK=0 P=50 Name=greater L=57 PPK=2
FakeLParens= FakeRParens=0 II=0x0 Text='>'
M=0 C=1 T=Unknown S=1 B=0 BK=0 P=50 Name=numeric_constant L=59 PPK=2
FakeLParens= FakeRParens=4 II=0x0 Text='1'
M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=r_paren L=60 PPK=2 FakeLParens=
FakeRParens=0 II=0x0 Text=')'
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79293
Files:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7065,6 +7065,8 @@
verifyFormat("static_assert(is_convertible<A &&, B>::value, \"AAA\");");
verifyFormat("Constructor(A... a) : a_(X<A>{std::forward<A>(a)}...) {}");
verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
+
+ verifyFormat("for ( unsigned i = 0; i < i; ++i, v = v >> 1 )");
}
TEST_F(FormatTest, BitshiftOperatorWidth) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -145,6 +145,10 @@
Contexts[Contexts.size() - 2].IsExpression &&
!Line.startsWith(tok::kw_template))
return false;
+ // If we see a ; then likely this is a for loop and not the template
+ if (CurrentToken->is(tok::semi))
+ return false;
+
updateParameterCount(Left, CurrentToken);
if (Style.Language == FormatStyle::LK_Proto) {
if (FormatToken *Previous = CurrentToken->getPreviousNonComment()) {
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7065,6 +7065,8 @@
verifyFormat("static_assert(is_convertible<A &&, B>::value, \"AAA\");");
verifyFormat("Constructor(A... a) : a_(X<A>{std::forward<A>(a)}...) {}");
verifyFormat("< < < < < < < < < < < < < < < < < < < < < < < < < < < < < <");
+
+ verifyFormat("for ( unsigned i = 0; i < i; ++i, v = v >> 1 )");
}
TEST_F(FormatTest, BitshiftOperatorWidth) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -145,6 +145,10 @@
Contexts[Contexts.size() - 2].IsExpression &&
!Line.startsWith(tok::kw_template))
return false;
+ // If we see a ; then likely this is a for loop and not the template
+ if (CurrentToken->is(tok::semi))
+ return false;
+
updateParameterCount(Left, CurrentToken);
if (Style.Language == FormatStyle::LK_Proto) {
if (FormatToken *Previous = CurrentToken->getPreviousNonComment()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
