rsmith added inline comments.
================
Comment at: clang/lib/AST/Expr.cpp:319
case RSK_None:
return;
case RSK_Int64:
----------------
Can you use `llvm_unreachable` here? (Are there cases where we use `RSK_None`
and then later find we actually have a value to store into the `ConstantExpr`?)
================
Comment at: clang/lib/Serialization/ASTReader.cpp:9635
+ if (IsExpr) {
+ Base = APValue::LValueBase(ReadExpr(F), CallIndex, Version);
+ ElemTy = Base.get<const Expr *>()->getType();
----------------
This is problematic.
`ReadExpr` will read a new copy of the expression, creating a distinct object.
But in the case where we reach this when deserializing (for a
`MaterializeTemporaryExpr`), we need to refer to the existing
`MaterializeTemporaryExpr` in the initializer of its lifetime-extending
declaration. We will also need to serialize the `ASTContext`'s
`MaterializedTemporaryValues` collection so that the temporaries
lifetime-extended in a constant initializer get properly handled.
That all sounds very messy, so I think we should reconsider the model that we
use for lifetime-extended materialized temporaries. As a half-baked idea:
* When we lifetime-extend a temporary, create a `MaterializedTemporaryDecl` to
hold its value, and modify `MaterializeTemporaryExpr` to refer to the
`MaterializedTemporaryDecl` rather than to just hold the subexpression for the
temporary.
* Change the `LValueBase` representation to denote the declaration rather than
the expression.
* Store the constant evaluated value for a materialized temporary on the
`MaterializedTemporaryDecl` rather than on a side-table in the `ASTContext`.
With that done, we should verify that all remaining `Expr*`s used as
`LValueBase`s are either only transiently used during evaluation or don't have
these kinds of identity problems.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63640/new/
https://reviews.llvm.org/D63640
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
