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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/charconv:147-151
+inline _LIBCPP_INLINE_VISIBILITY chars_format& operator^=(chars_format& __x,
+                                                          chars_format __y) {
+  __x = __x ^ __y;
+  return __x;
+}
----------------
Mordante wrote:
> Quuxplusone wrote:
> > This should also be `constexpr`, unless I'm mistaken. Even if it's not mandated, certainly it would be //nice QoI// to be able to use `chars_format x, y; x ^= y` in a constexpr function the same way we can use `int a, b; a ^= b` today.
> They shouldn't be `constexpr` per http://eel.is/c++draft/bitmask.types#2.
> My guess is the bitmask type was added/updated in C++11. IIRC in C++11 a `constexpr` could only have a single `return` and they have never been updated. Guess it would be nice to write a proposal to update this type.
> 
> I agree it would be nice to use `constexpr`, but that's not allowed per http://eel.is/c++draft/constexpr.functions.
The code you quoted is just "exposition-only"; I think the normative text in http://eel.is/c++draft/bitmask.types#1 takes precedence, and that normative text clearly states that integer types //are// bitmask types. Integer types have constexpr assignment ops. Thus, bitmask types //can// have constexpr assignment ops, even if the exposition-only code does depict a specific example type without constexpr support.

Right now, libc++ is the only one of the Big Three to provide compound operators that don't work in constexpr functions. Let's fix that bug and help users write more portable code. https://godbolt.org/z/r7fzo4oKh


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97115