[PATCH] D131479: Handle explicitly defaulted consteval special members.

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 29 05:21:49 PDT 2022


aaron.ballman added a comment.

In D131479#3753586 <https://reviews.llvm.org/D131479#3753586>, @Mordante wrote:

> In D131479#3753533 <https://reviews.llvm.org/D131479#3753533>, @aaron.ballman wrote:
>
>> In D131479#3753462 <https://reviews.llvm.org/D131479#3753462>, @Mordante wrote:
>>
>>> This change breaks the libc++ test `libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp`. (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)
>>>
>>> Looking at the description of this patch it should only affect `consteval` functions, but it seems to affect `constexpr` functions too. Can you have a look?
>>
>> Agreed that we should take a look, but it's interesting to note that libstdc++ seems to have a different behavior than libc++ here: https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that the libc++ test is actually correct, because (without checking the standard, so take with a giant grain of salt) this seems like it might have fixed a bug in libc++? The release note says this is expected to impact constexpr and consteval default special member functions, so behavioral changes to `constexpr` aren't unexpected (though I agree that the commit message didn't mention `constexpr` so the changes may seem surprising).
>
> Thanks for confirming it's indeed intended to affect `constexpr` too! It was a bit confusing since it also referred to C++14, which didn't have `consteval`. I wasn't entirely sure about the status of libc++, but since the patch gave of a mixed message I wanted make sure that `constexpr` was intended.

It was a good question to bring up!

> I've done some further digging and according to cppreference the paper P0602R4 was a DR against C++20. This was proposed in the paper.
> Looking at the changes in the addressing this paper I see no C++ version checks https://reviews.llvm.org/D32385
> In a followup introduces tests using C++ version checks https://reviews.llvm.org/D54772
> The failing test was introduced in 308624127fd6cc36558b6eee4d4ffa4e215a074e before the paper was voted in
>
> So I think there's indeed a bug in libc++ which was hidden since Clang hadn't implemented some DRs. I will look at the libc++ side what needs to be done to fix the CI failures.

Great sleuth work! Thank you for the investigation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479



More information about the cfe-commits mailing list