[C++20] Fix crash-on-valid with consteval temporary construction through list initialization

[aaron.ballman]

Summary

Clang currently crashes when lowering a consteval list initialization of a temporary. This is partially working around an issue in the template instantiation code (TreeTransform::TransformCXXTemporaryObjectExpr()) that does not yet know how to handle list initialization of temporaries in all cases. However, it's also helping reduce fragility by ensuring we always have a valid QualType when trying to emit a constant expression during IR generation.

Fixes #55871

[erichkeane]

clang/lib/CodeGen/CGExprConstant.cpp

1404

Is there any possibility to have an assert somewhere to catch when this ISN'T the type we need to see later? It would be nice to figure out what other cases we're missing below.

[aaron.ballman]

clang/lib/CodeGen/CGExprConstant.cpp

1404

Not that I've found yet, but if someone has ideas, I'm definitely open to them because I worry about that as well. I had originally tried just asserting that the value was valid, but a *bunch* of code calls this where the RetType doesn't matter and was tripping up that assertion.

[erichkeane]

clang/lib/CodeGen/CGExprConstant.cpp

1404

Hrmph... ok. I was hoping that if we 'messed up' the return type here, AND it was useful later, it would be 'caught' by a later assert. Or was something like that (Like in the CXXFunctionalCastExpr case?) already asserting in a callee?

[efriedma]

Instead of trying to dig through subexpressions of the ConstantExpr to try to infer the type we need, can we just compute it directly?

QualType RetType = CE->getType();
if (CE->isGLValue())
  RetType = CGM.getContext().getLValueReferenceType(RetType);

[aaron.ballman]

Improved from review feedback.

[aaron.ballman]

In D131194#3700688, @efriedma wrote:

Instead of trying to dig through subexpressions of the ConstantExpr to try to infer the type we need, can we just compute it directly?

QualType RetType = CE->getType();
if (CE->isGLValue())
  RetType = CGM.getContext().getLValueReferenceType(RetType);

Oh wow, that's a *much* better approach, thank you for the suggestion! It passed my local testing, so hopefully precommit CI doesn't spot any concerns from it.

[aaron.ballman]

Ping

[efriedma]

LGTM

clang/lib/CodeGen/CGExprConstant.cpp

1402

Not sure the assertion is useful; RetType is obviously non-null in the current version.

[aaron.ballman]

clang/lib/CodeGen/CGExprConstant.cpp

1402

Fair enough, I removed it when landing the changes.

clang/docs/ReleaseNotes.rst

 - Fix `#56772 <https://github.com/llvm/llvm-project/issues/56772>`_ - invalid
   destructor names were incorrectly accepted on template classes.
 - Improve compile-times with large dynamic array allocations with trivial
   constructors. This fixes
   `Issue 56774 <https://github.com/llvm/llvm-project/issues/56774>`_.
 - No longer assert/miscompile when trying to make a vectorized ``_BitInt`` type
   using the ``ext_vector_type`` attribute (the ``vector_size`` attribute was
   already properly diagnosing this case).
+- Fixed a crash-on-valid with consteval evaluation of a list-initialized
+  constructor for a temporary object. This fixes
+  `Issue 55871 <https://github.com/llvm/llvm-project/issues/55871>`_.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang will now correctly diagnose as ill-formed a constant expression where an
   enum without a fixed underlying type is set to a value outside the range of
   the enumeration's values. Fixes
   `Issue 50055: <https://github.com/llvm/llvm-project/issues/50055>`_.
 - Clang will now check compile-time determinable string literals as format strings.

clang/lib/CodeGen/CGExprConstant.cpp

   auto C = tryEmitPrivate(value, destType);
   return validateAndPopAbstract(C, state);
 }
 
 llvm::Constant *ConstantEmitter::tryEmitConstantExpr(const ConstantExpr *CE) {
   if (!CE->hasAPValueResult())
     return nullptr;
   const Expr *Inner = CE->getSubExpr()->IgnoreImplicit();
-  QualType RetType;
+
+  // In a call below, emitAbstract() calls tryEmitPrivate() and *some parts* of
+  // that call will use the RetType we pass in here. The parts which do make
+  // use of it expect that the QualType is valid, which is why we give it a
+  // default value here and reset the type only as needed.
+  QualType RetType = Inner->getType();
   if (auto *Call = dyn_cast<CallExpr>(Inner))
     RetType = Call->getCallReturnType(CGM.getContext());
   else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Inner))
     RetType = Ctor->getType();
+  else if (auto *Cast = dyn_cast<CXXFunctionalCastExpr>(Inner)) {
+    // FIXME: This is working around a deficiency in template instantiation;
+    // see TreeTransform::TransformCXXTemporaryObjectExpr and GitHub Issue
+    // #55871 for more details.
+    RetType = Cast->getType();
+  }
+
+  assert(!RetType.isNull() && "Not certain of the constant expression's type");
   llvm::Constant *Res =
       emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
   return Res;
 }
 
 llvm::Constant *
 ConstantEmitter::emitAbstract(const Expr *E, QualType destType) {
   auto state = pushAbstract();

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

 
 // CHECK-NOT: define{{.*}} zeroext i8 @_ZN10Issue54578li3_UCEy
 // CHECK: define{{.*}} i32 @_ZN10Issue545783fooEv(
 // CHECK: define{{.*}} signext i8 @_ZN10Issue545782f1IcEEcT_(
 // CHECK: ret i8 4
 // CHECK: define{{.*}} signext i8 @_ZN10Issue545782f2IcEEcT_(
 // CHECK: ret i8 4
 }
+
+namespace Issue55871 {
+struct Item {
+  consteval Item(char c) :_char{c}{}
+  char _char;
+};
+
+int function(const Item& item1, const Item& item2) {
+  return 0;
+}
+
+int foo() {
+  return function(Item{'a'}, Item{'a'});
+}
+} // namespace Issue58871