Page MenuHomePhabricator

[libc++] [LIBCXX-DEBUG-FIXME] Constexpr char_traits::copy mustn't compare unrelated pointers.
Changes PlannedPublic

Authored by Quuxplusone on Apr 30 2021, 4:50 PM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Summary
We could arrange to skip the UB-inducing assert in the case where the computation
might be happening at constexpr time; but we have no existing way to check for
computation that *might be* happening at constexpr time. We only have a way
to check for computation that is *definitely* happening at constexpr time
(versus *might be* happening at runtime). __libcpp_is_constant_evaluated()
returns true only if we KNOW it's constexpr-time; it returns false both at runtime
and on compilers that don't support the builtin.
Come to think of it, maybe this is the wrong default for __libcpp_is_constant_evaluated().

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Apr 30 2021, 4:50 PM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptApr 30 2021, 4:50 PM

poke buildkite

ldionne requested changes to this revision.May 5 2021, 10:53 AM

We should be able to compare pointers with std::less to solve this problem. It won't work if you just start using std::less because our std::less is actually wrong, however that's another problem we need to solve. We shouldn't just remove this assertion and hide the problem. So:

  1. Fix std::less so that it's not UB to use it to compare unrelated pointers (will require having some sort of special contract with the compiler that std::less can do that -- this could be done with some sort of builtin).
  2. Fix the debug mode so that it allows comparing unrelated pointers with std::less (frankly I'm not sure what's the best way of doing that off the top of my head, but we can explore).
  3. Use std::less to implement those pointer comparisons.

WDYT?

This revision now requires changes to proceed.May 5 2021, 10:53 AM
Quuxplusone planned changes to this revision.May 5 2021, 4:34 PM

We should be able to compare pointers with std::less to solve this problem. [...]
(will require having some sort of special contract with the compiler that std::less can do that -- this could be done with some sort of builtin)

I agree that that's what WG21 seems to want vendors to do. But that requires coordinating a new pointer-comparison builtin with both Clang and GCC, which is (definitely a good idea but) far above my pay grade. I'm happy to abandon this PR and let someone-not-me pursue the builtin angle...

Discussed offline with @ldionne and decided to sit on this for now, with the intent that someone will pursue adding a builtin at least to Clang.