[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal
Jan Korous via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 5 11:20:18 PST 2019
jkorous accepted this revision.
jkorous added a comment.
This revision is now accepted and ready to land.
In D58922#1418029 <https://reviews.llvm.org/D58922#1418029>, @MyDeveloperDay wrote:
> do you mean this case? as this seems to work for me?
>
> verifyFormat("namespace bar {\n"
> "auto foo{[]() -> foo<false> { ; }};\n"
> "} // namespace bar");
>
I meant that if you take the test and run it on unpatched master (simulating hypothetical test failure in the future) the message it produces starts by stating that the expected string (part of the testcase) is "unstable" which seems a bit unclear/confusing to me.
/Users/jankorous/src/llvm-monorepo/llvm-project/clang/unittests/Format/FormatTest.cpp:74: Failure
Expected: Expected.str()
Which is: "namespace bar {\nauto foo{[]() -> foo<false> { ; }};\n} // namespace bar"
To be equal to: format(Expected, Style)
Which is: "namespace bar {\nauto foo{[]() -> foo<false>{;\n}\n}\n;\n} // namespace bar"
With diff:
@@ -1,3 +1,6 @@
namespace bar {
-auto foo{[]() -> foo<false> { ; }};
+auto foo{[]() -> foo<false>{;
+}
+}
+;
} // namespace bar
Whereas the full original reproducer (with the "//// broken:" comment included) also starts with the same message but the diff implies that the implementation is wrong by duplicating the "//// namespace bar" comment.
/Users/jankorous/src/llvm-monorepo/llvm-project/clang/unittests/Format/FormatTest.cpp:74: Failure
Expected: Expected.str()
Which is: "namespace bar {\n// broken:\nauto foo{[]() -> foo<5 + 2> { return {}; }};\n} // namespace bar"
To be equal to: format(Expected, Style)
Which is: "namespace bar {\n// broken:\nauto foo{[]() -> foo<5 + 2>{return {};\n} // namespace bar\n}\n;\n} // namespace bar"
With diff:
@@ -1,4 +1,7 @@
namespace bar {
// broken:
-auto foo{[]() -> foo<5 + 2> { return {}; }};
+auto foo{[]() -> foo<5 + 2>{return {};
+} // namespace bar
+}
+;
} // namespace bar
Anyway. LGTM.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58922/new/
https://reviews.llvm.org/D58922
More information about the cfe-commits
mailing list