[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

 - 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).
 - Fix clang not properly diagnosing the failing subexpression when chained
   binary operators are used in a ``static_assert`` expression.
 - Fix a crash when evaluating a multi-dimensional array's array filler
   expression is element-dependent. This fixes
   `Issue 50601 <https://github.com/llvm/llvm-project/issues/56016>`_.
-
+- 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. Due to the extended period of time this bug was
   present in major C++ implementations (including Clang), this error has the
   ability to be downgraded into a warning (via: -Wno-error=enum-constexpr-conversion)

clang/lib/CodeGen/CGExprConstant.cpp

ConstantEmitter::tryEmitAbstract(const APValue &value, QualType destType) {
   auto state = pushAbstract();
   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;
-  if (auto *Call = dyn_cast<CallExpr>(Inner))
-    RetType = Call->getCallReturnType(CGM.getContext());
-  else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Inner))
-    RetType = Ctor->getType();
-  llvm::Constant *Res =
-      emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
-  return Res;
+
+  QualType RetType = CE->getType();
+  if (CE->isGLValue())
+    RetType = CGM.getContext().getLValueReferenceType(RetType);
+
+  return emitAbstract(CE->getBeginLoc(), CE->getAPValueResult(), RetType);
 }
 
 llvm::Constant *
 ConstantEmitter::emitAbstract(const Expr *E, QualType destType) {
   auto state = pushAbstract();
   auto C = tryEmitPrivate(E, destType);
   C = validateAndPopAbstract(C, state);
   if (!C) {
     CGM.Error(E->getExprLoc(),

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