[clang] Define the __cpp_consteval macro now that support is complete

[BertalanD]

Commit 3d2629d marked consteval support as completed for the Clang 15 release in the docs. However, the __cpp_consteval feature test macro is still not defined. If the feature is indeed production-ready, it would be nice to define this macro, both on main and the release branch.

llvm-project/clang/lib/Frontend/InitPreprocessor.cpp

Line 679 in 0b12f77

//Builder.defineMacro("__cpp_consteval", "201811L");

cc @cor3ntin, @ChuanqiXu9

See also #53906

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[cor3ntin]

There are still some bugs in the implementation so i think it made sense to be conservative in 15 but we should strive to add the macro in clang 16.

@AaronBallman

[llvmbot]

@llvm/issue-subscribers-c-20

[AaronBallman]

There are still some bugs in the implementation so i think it made sense to be conservative in 15 but we should strive to add the macro in clang 16.

@AaronBallman

I agree -- we're close to being complete, but I think there's still some outstanding bugs we should address before we provide the macro. That said, we might discover new bugs by enabling the macro and trying out some large projects (in case anyone is looking for an experiment to run and report back on).

[AaronBallman]

I did some investigating and we're not as close as I'd like to be. Specifically, there are a few outstanding cases from P1073R3 that we don't handle properly:

namespace P1073R3 {
consteval int f() { return 42; } // expected-note 3 {{declared here}}
consteval auto g() { return f; }
// FIXME: there should be no diagnostics associated with either h() or r.
consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
                                                     expected-note {{declared here}} \
                                                     expected-note {{pointer to a consteval declaration is not a constant expression}}
constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
constexpr auto e = g();  // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
                            expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
                            expected-note 2 {{pointer to a consteval declaration is not a constant expression}}
} // namespace P1073R3

namespace P1073R3 {
struct N {
  constexpr N() {}
  N(N const&) = delete;
};

template<typename T> constexpr void bad_assert_copyable() { T t; T t2 = t; }
using ineffective = decltype(bad_assert_copyable<N>());

// bad_assert_copyable<N> is not needed for constant evaluation
// (and thus not instantiated)
template<typename T> consteval void assert_copyable() { T t; T t2 = t; }
using check = decltype(assert_copyable<N>());
// FIXME: this should give an error because assert_copyable<N> is instantiated
// (because it is needed for constant evaluation), but the attempt to copy t is
// ill-formed.
} // namespace P1073R3

There is some implementation divergence, but I believe Clang's behavior is the least conforming of the implementations tested: https://godbolt.org/z/soMs4aWGo

[AaronBallman]

FWIW, I posted https://reviews.llvm.org/D144572 to try to update the public status page and get more info into the test suite as to what functionality is still missing before we should claim full support (IMO).

[Fznamznon]

Specifically, there are a few outstanding cases from P1073R3 that we don't handle properly

I'm looking into these.

[Fznamznon]

Here is an attempt to fix the first one - https://reviews.llvm.org/D145251 .

[Fznamznon]

Regarding the following example:

namespace P1073R3 {
struct N {
  constexpr N() {}
  N(N const&) = delete;
};

template<typename T> constexpr void bad_assert_copyable() { T t; T t2 = t; }
using ineffective = decltype(bad_assert_copyable<N>());

// bad_assert_copyable<N> is not needed for constant evaluation
// (and thus not instantiated)
template<typename T> consteval void assert_copyable() { T t; T t2 = t; }
using check = decltype(assert_copyable<N>());
// FIXME: this should give an error because assert_copyable<N> is instantiated
// (because it is needed for constant evaluation), but the attempt to copy t is
// ill-formed.
} // namespace P1073R3

I think clang is correct here. https://wg21.link/p1937 proposes that in unevaluated contexts, consteval
functions should not be immediately evaluated. Clang implemented p1937 a while ago - 131b462 , so its behavior is correct here. I posted https://reviews.llvm.org/D145362 to update the test. It also adds examples from p1937.

@AaronBallman , is my understanding correct once the two outstanding cases (#57094 (comment) ) are fixed, clang can claim support for consteval again?
I do realize there is some bugs still, but where is the place without bugs? :D

[AaronBallman]

Regarding the following example:

namespace P1073R3 {
struct N {
  constexpr N() {}
  N(N const&) = delete;
};

template<typename T> constexpr void bad_assert_copyable() { T t; T t2 = t; }
using ineffective = decltype(bad_assert_copyable<N>());

// bad_assert_copyable<N> is not needed for constant evaluation
// (and thus not instantiated)
template<typename T> consteval void assert_copyable() { T t; T t2 = t; }
using check = decltype(assert_copyable<N>());
// FIXME: this should give an error because assert_copyable<N> is instantiated
// (because it is needed for constant evaluation), but the attempt to copy t is
// ill-formed.
} // namespace P1073R3

I think clang is correct here. https://wg21.link/p1937 proposes that in unevaluated contexts, consteval functions should not be immediately evaluated. Clang implemented p1937 a while ago - 131b462 , so its behavior is correct here. I posted https://reviews.llvm.org/D145362 to update the test. It also adds examples from p1937.

You're correct, good catch!

@AaronBallman , is my understanding correct once the two outstanding cases (#57094 (comment) ) are fixed, clang can claim support for consteval again? I do realize there is some bugs still, but where is the place without bugs? :D

I think so. Looking through our open issues, I'm not seeing anything that seems like a blocker (though I am seeing plenty of bugs we need to address) and I'm not aware of any outstanding C++20 era issues. It might be nice to ensure the consteval examples from the C++20 standard behave as expected (taking DRs into account), just to be sure.

[Fznamznon]

I submitted https://reviews.llvm.org/D145251 and https://reviews.llvm.org/D145362 , looking if there is any remaining problems with examples from the standard.

[AaronBallman]

I submitted https://reviews.llvm.org/D145251 and https://reviews.llvm.org/D145362 , looking if there is any remaining problems with examples from the standard.

I just checked the standard and didn't spot any other examples we weren't handling properly. I also looked through the open issues in Clang, and here are the ones that concern me still:

#60286 (missing copy elision during constant evaluation, rejects valid code)
#58207 (failed assertion due to invalid internal state)

There were a few others that could be worrisome, but they need further reduction and analysis to be sure. IMO, we don't have to be bug-free but we shouldn't knowingly reject or crash on valid code either. AFAIK, those are the only two outstanding issues that I think qualify as blockers for the feature test macro. If others know of issues they think should also block this, please speak up!

[Fznamznon]

I also looked through the open issues in Clang, and here are the ones that concern me still:
#60286 (missing copy elision during constant evaluation, rejects valid code)
#58207 (failed assertion due to invalid internal state)

Ok, I'm looking into #58207 .

[h-vetinari]

I submitted https://reviews.llvm.org/D145251 and https://reviews.llvm.org/D145362 , looking if there is any remaining problems with examples from the standard.

Notwithstanding the definition of the macro, it seems that after these PRs, the status of P1073R3 on https://clang.llvm.org/cxx_status.html should go back to "Full" (resp. "Unreleased")?

(I'm writing here as there's no separate issue for P1073 AFAICT)

[AaronBallman]

I submitted https://reviews.llvm.org/D145251 and https://reviews.llvm.org/D145362 , looking if there is any remaining problems with examples from the standard.

Notwithstanding the definition of the macro, it seems that after these PRs, the status of P1073R3 on https://clang.llvm.org/cxx_status.html should go back to "Full" (resp. "Unreleased")?

(I'm writing here as there's no separate issue for P1073 AFAICT)

Yes, in general I think the feature test macro and the feature status page should be in agreement. So once we define the macro, we should claim support on the webpage.

[h-vetinari]

My point was that (IIUC...) P1073 has a much smaller scope than all of C++20 __cpp_consteval; the latter may not be ready, the former should now be.

[AaronBallman]

Is it? Our status page says (in part) "Clang still incorrectly defers some consteval executions to runtime" and I think we have several open issues about that:

#61142
#59447
#58207

(probably others). But if we think we've tracked those down already, maybe we can mark P1073 as full now.

[h-vetinari]

But if we think we've tracked those down already, maybe we can mark P1073 as full now.

TBH, I was naïvely going by the tests you had added in https://reviews.llvm.org/D144572, which have now been fixed. But that's possibly because I had the apparently wrong impression that the __cpp_consteval macro covers a superset of P1073.

[AaronBallman]

But if we think we've tracked those down already, maybe we can mark P1073 as full now.

TBH, I was naïvely going by the tests you had added in https://reviews.llvm.org/D144572, which have now been fixed. But that's possibly because I had the apparently wrong impression that the __cpp_consteval macro covers a superset of P1073.

Well, it sort of does, but here's my thinking: P1073 is what first introduced the idea of consteval functions and the Big Idea from that feature is to force expression evaluation to never be deferred to runtime (always computed at compile time or you get an error). So because we still defer some consteval evaluations to runtime accidentally, it doesn't seem like we should claim to support P1073 quite yet. But it's a value judgement because bug-free is an almost unattainable bar -- so if someone has a counter-argument, we should definitely explore it.

[Fznamznon]

So, a little update
I've submitted fixes for
#61142
#58207 (and #59447 since this is technically a duplicate).
#60286

@AaronBallman , do you think we're ready to claim the full support again?

[AaronBallman]

So, a little update I've submitted fixes for #61142 #58207 (and #59447 since this is technically a duplicate). #60286

Thank you for these fixes!

@AaronBallman , do you think we're ready to claim the full support again?

Yes, I think we are. Would you mind putting up a patch to define __cpp_consteval and update the www/cxx_status.html page?

[Fznamznon]

Yes, I think we are. Would you mind putting up a patch to define __cpp_consteval and update the www/cxx_status.html page?

Sure!
UPD: here it is https://reviews.llvm.org/D147717 .