[libcxx-commits] [PATCH] D96664: [libc++][format] Implement formatters.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


================
Comment at: libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.bool.pass.cpp:38
+  static_assert(std::semiregular<decltype(formatter)>);
+  formatter.parse(parse_ctx);
+
----------------
I'd add a check that the return value points to the end of the format string here and elsewhere.


================
Comment at: libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.bool.pass.cpp:51-52
+void test() {
+  test(STR("1"), STR("}"), true);
+  test(STR("0"), STR("}"), false);
+}
----------------
I'd test that empty string is also parsed correctly which is an important corner case.


================
Comment at: libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.floating_point.pass.cpp:58
+  using A = Arithmetic;
+  test(STR("{}"), A(-std::numeric_limits<float>::max()));
+  test(STR("{}"), A(-std::numeric_limits<float>::min()));
----------------
Why do you have "}" in other tests and "{}" here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96664/new/

https://reviews.llvm.org/D96664