Page MenuHomePhabricator

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

Authored by aaron.ballman on Aug 4 2022, 12:24 PM.

Details

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

Diff Detail

Event Timeline

aaron.ballman created this revision.Aug 4 2022, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 12:24 PM
aaron.ballman requested review of this revision.Aug 4 2022, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2022, 12:24 PM
erichkeane added inline comments.Aug 4 2022, 12:30 PM
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 added inline comments.Aug 4 2022, 12:32 PM
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 added inline comments.Aug 4 2022, 12:39 PM
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?

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);

Improved from review feedback.

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 marked 2 inline comments as done.Aug 11 2022, 4:55 AM

Ping

efriedma accepted this revision as: efriedma.Aug 11 2022, 10:15 AM

LGTM

clang/lib/CodeGen/CGExprConstant.cpp
1402

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

This revision is now accepted and ready to land.Aug 11 2022, 10:15 AM
aaron.ballman added inline comments.Aug 11 2022, 10:44 AM
clang/lib/CodeGen/CGExprConstant.cpp
1402

Fair enough, I removed it when landing the changes.