Constant expression evaluation misses copy elision

[AaronBallman]

The following code fails to compile on Clang:

struct A {
    int i = 0;

    consteval A() {}
    A(const A&) { i = 1; }
    consteval int f() { return i; }
};

int main() {
    return A{A{}}.f();
}

because we are consider the copy constructor call to be invalid due to not being consteval.

https://gcc.godbolt.org/z/P8xYjnYjM

[llvmbot]

@llvm/issue-subscribers-c-20

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[Fedr]

I think in your example A{A{}}.f() shall return 0 (copy is elided). Online demo: https://gcc.godbolt.org/z/rxdWsv6s8

But this is different from the original report, where the issue happens without static_assert: #53244

[AaronBallman]

Ugh, yeah, I mucked up the example code in the summary. The crux of the example at https://gcc.godbolt.org/z/sn68z5Gaf is this bit:

<source>:19:14: note: non-constexpr constructor 'A' cannot be used in a constant expression
    return A{A{}}.f();
             ^
<source>:11:5: note: declared here
    A(const A&) { i = 1; }
    ^

we should not be noting that constructor because it should have been elided, therefore:

<source>:19:12: error: call to consteval function 'A::f' is not a constant expression
    return A{A{}}.f();
           ^

should have not been diagnosed. I think our constant expression evaluator is getting confused.

[zygoloid]

Note that copy elision is not performed as part of constant evaluation. If there is a bug here, the bug would be that we should not be calling a copy constructor at all as part of initialization.

[AaronBallman]

Oh, thank you for pointing that out Richard, I had completely missed the bit you linked.

[Fznamznon]

I've spent some time looking at this. It seems that when a potential immediate invocation is met, it is immediately wrapped by a ConstantExpr but not yet evaluated. There is also a TreeTransform that removes this ConstantExpr wrapper when corresponding expression evaluation context is popped.
There is a "strange" (according to the comments) place where CXXFunctionalCastExpr is added to AST:

llvm-project/clang/lib/Sema/SemaExprCXX.cpp

Line 1607 in 553bff0

Result = CXXFunctionalCastExpr::Create(

Basically there is a check that if CXXTemporaryObjectExpr was created, then no need to add an additional CXXFunctionalCastExpr. However due to default constructor being consteval and therefore its use being an immediate invocation, there was CXXTemporaryObjectExpr but wrapped with ConstantExpr representing an immediate invocation (mentioned above). That caused adding additional CXXFunctionalCastExpr and this CXXFunctionalCastExpr being in place caused the TreeTransform that happens at place where immediate invocations are handled (also mentioned above) to add constructor call causing the error. So, basically, adding something like

diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e7f3852ae34c..d687e87fcb0e 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1590,6 +1590,9 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
   Expr *Inner = Result.get();
   if (CXXBindTemporaryExpr *BTE = dyn_cast_or_null<CXXBindTemporaryExpr>(Inner))
     Inner = BTE->getSubExpr();
+  if (ConstantExpr *CE = dyn_cast_or_null<ConstantExpr>(Inner))
+    if (CE->isImmediateInvocation())
+      Inner = CE->getSubExpr();
   if (!isa<CXXTemporaryObjectExpr>(Inner) &&
       !isa<CXXScalarValueInitExpr>(Inner)) {
     // If we created a CXXTemporaryObjectExpr, that node also represents the

Helps to stop diagnosing this code, which considered as valid by other compilers. If that sounds ok, I can put the patch for review.

[Fznamznon]

https://reviews.llvm.org/D146801 .