Quuxplusone added inline comments.
================
Comment at: libcxx/include/bit:378
+ const unsigned __retVal = 1u << (__n + __extra);
+ return (_Tp) (__retVal >> __extra);
+ }
----------------
mclow.lists wrote:
> mclow.lists wrote:
> > Quuxplusone wrote:
> > > Why so complicated? Is there a unit test that demonstrates why you can't
> > > just use `return _Tp{1} << __n;` in this case as well?
> > >
> > > Also, is this a kosher use of `sizeof(X) * 8` as a stand-in for
> > > `numeric_limits<X>::digits`?
> > >
> > > Also, speaking of unit tests, I don't see any unit tests for e.g.
> > > `std::ceil2(256)` or `std::ceil2(65536)`. Shouldn't there be some?
> > Yes. I want to generate some UB here, so that this is not a "core constant
> > expression" as per P1355.
> Yes, the `ceil2.fail.cpp` test will not fail (for short types) if I just
> return `_Tp{1} << __n;` - because of integer promotion.
>
Yikes. Sounds like there's some "library maintainer"ship going on here, then.
Naively, I would have thought that the Working Draft had left the behavior of
such constructs undefined in order to make the library vendor's life **easier**
and the code **more efficient**. But you're saying that you need to pessimize
the code here in order to make it "sufficiently undefined" so that its behavior
at compile time will be well-defined and produce a diagnostic instead of being
undefined and producing garbage?
Maybe WG21 needs to invent a "really actually undefined behavior" that does not
have unwanted interactions with constexpr, so that library writers can go back
to generating fast clean code!
Rant aside, why don't you just do `_LIBCPP_ASSERT(__n <
numeric_limits<_Tp>::digits);` and leave it at that? An assertion failure
isn't a compile-time constant, is it?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51262/new/
https://reviews.llvm.org/D51262
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
