[libcxx-commits] [PATCH] D60368: Add bind_front function (P0356R5)
Zoe Carver via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Feb 9 11:22:01 PST 2021
zoecarver added inline comments.
================
Comment at: libcxx/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp:149
#if defined(_LIBCPP_VERSION)
- ret = ret2;
- assert(ret() == true);
- assert(ret2() == true);
+ if (!std::is_constant_evaluated()) {
+ ret = ret2;
----------------
ldionne wrote:
> Quuxplusone wrote:
> > zoecarver wrote:
> > > @ldionne I did have one problem after rebasing. Because this is implemented with `std::tuple`, the assignment operator isn't constexpr. I see a few options here:
> > >
> > > 1. Wait to land this until we implement constexpr assignment operators for `std::tuple`.
> > > 2. Add the above check (`is_constant_evaluated`) to this test and remove it once constexpr assignment operators for `std::tuple` are implemented.
> > > 3. Just remove this part of the test entirely because it's not required by the standard.
> > > 4. Update `std::tuple` to have constexpr assignment operators (without properly implementing the paper).
> > FWIW, I vote for #3 — `std::not_fn(x)` isn't supposed to be assignable. It might even be user-friendly to //delete// its assignment operator so that libc++ users don't start relying on it and then break when they move to a different STL vendor.
> > Alternatively, provide and test the assignment operator but move the test into `test/libcxx/utilities/` instead??
> Yeah, I agree with Arthur about doing (3).
OK. Want me to delete the assignment operators? I don't care one way or another, but it might break some people who are (unfortunately) relying on it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60368/new/
https://reviews.llvm.org/D60368
More information about the libcxx-commits
mailing list