[libcxx-commits] [PATCH] D93166: [libc++][format] Add basic_format_parse_context.

Marek Kurdej via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Dec 15 13:03:28 PST 2020

curdeius accepted this revision.
curdeius added a comment.

Apart from the question whether adding assertion fits here, this LGTM.

Comment at: libcxx/test/std/utilities/format/format.formatter/format.parse.ctx/check_arg_id.pass.cpp:64
+  test();
+  test_exception();
+  static_assert(test());
Mordante wrote:
> curdeius wrote:
> > I don't see why you can't test the first part of `test_exception` in constexpr context. Changing return type to `bool` and returning false/true where appropriate should be enough to make it work.
> > Obviously, the part with `test_arg` can't be tested there.
> I'm not entirely sure what you mean. The line `context.next_arg_id();` will call `__format::__throw_error` which is not a `constexpr` function, turning it into one will fail with the following diagnostic ` error: constexpr function never produces a constant expression [-Winvalid-constexpr]`. Can you explain what change you think should work.
> I made the separate function `test_exception` for the parts which can't be tested as `constexpr`.
You're completely right. I must have been confused when writing this:).

Comment at: libcxx/test/std/utilities/format/format.formatter/format.parse.ctx/next_arg_id.pass.cpp:54
+  test();
+  test_exception();
+  static_assert(test());
curdeius wrote:
> As above, but here you can test everything in constexpr context.
Please ignore the above too.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list