[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