[PATCH] D76447: Apply ConstantEvaluated evaluation contexts to more manifestly constant evaluated scopes

[Wyatt Childers via Phabricator via cfe-commits]

wchilders marked an inline comment as done.
wchilders added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:15597
+      (Rec.isConstantEvaluated() &&
+       Rec.ExprContext != ExpressionKind::EK_ConstexprVarInit)) {
     ExprCleanupObjects.erase(ExprCleanupObjects.begin() + Rec.NumCleanupObjects,
----------------
This is my main concern with this patch. There's something subtle here, and it's unclear from an outsider's (my) perspective what the root issue is here.

Effectively we get into trouble when we first enter `ActOnCXXEnterDeclInitializer` on the `isManifestlyEvaluatedVar` branch. We then exit at `ActOnCXXExitDeclInitializer`, triggering this branch, as `Rec.isConstantEvaluated()` is true. This results in the cleanups being dropped. Then when we enter the constexpr evaluator for `VarDecl::evaluateValue`. the evaluator is unhappy as we're missing a cleanup for the created temporary.

Any input that allows this to be resolved without the special case, or that otherwise informs how this should work, and why unevaluated and constant evaluated are treated equally here, would be awesome. I'll note that in reviewing the revision history, this condition dates back to when `ConstantEvaluated` was created, so perhaps this is a "historical artifact" of that time?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76447/new/

https://reviews.llvm.org/D76447