[libcxx-commits] [PATCH] D97115: [libc++] Make chars_format a bitmask type.

Thu Apr 8 11:11:21 PDT 2021

ldionne added inline comments.

================
Comment at: libcxx/test/std/utilities/charconv/charconv.syn/chars_format.pass.cpp:69
+         ((~std::chars_format::hex & std::chars_format::general) ==
+          std::chars_format::general);
+}
----------------
Mordante wrote:
> Quuxplusone wrote:
> > Mordante wrote:
> > > Quuxplusone wrote:
> > > > Please rewrite this function to use `assert(x); assert(y); assert(z); return true;`
> > > > instead of `return x && y && z;`
> > > I personally prefer to write the code as you suggested, in fact I originally wrote it like that. Unfortunately `assert` can't be used in a constexpr function in C++11. Hence this comment at line 48: "This construction is used to make the function constexpr in C++11."
> > <charconv> was introduced in C++17. Anyone still using C++11 constexpr in the year of our lord 2021 deserves what they get.
> > 
> > If you need to add `UNSUPPORTED: c++11` to this test in order to use the preferred style, I think that's a good tradeoff. @ldionne, thoughts?
> The reason I use C++11 in this test is the fact that libc++ has enabled `std::to_chars` in C++11 as an extension.
I think we're better off assuming C++17.

It looks like they discussed the issue of back-porting in https://reviews.llvm.org/D41458 (which added `to_chars`), but I'm not satisfied by that discussion. If I had commented on that review, I would have pushed back on supporting this pre-C++17 with all I have.

Now it's done, but at least I think it's clear that IMO it's reasonable not to bend backwards for the tests to support < C++17.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97115