rsmith added inline comments.
================ Comment at: clang/lib/AST/Expr.cpp:2875 - else if (auto *CE = dyn_cast<ConstantExpr>(E)) - return CE->getSubExpr(); - return E; } ---------------- wchilders wrote: > `IgnoreParensSingleStep` for some reason has been unwrapping `ConstantExpr`s. > This results in the constant evaluator, removing the ConstantExpr, and > reevaluating the expression. There are no observed downsides to removing this > condition, in the test suite, however, it's strange enough to note. We sadly don't really have any good, rigorous definitions of what the various `Ignore*` functions do. But the general idea of `IgnoreParens` is to ignore syntax with no semantic effect, and `ConstantExpr` doesn't describe syntax, so it probably shouldn't be stepped over here. (It is ignored by `IgnoreImplicit`, which is generally -- mostly -- trying to ignore semantics with no syntactic representation, which seems appropriate. It might make sense for `IgnoreImpCasts` to not step over `FullExpr`s, but I expect that change would break some testcases.) ================ Comment at: clang/lib/AST/ExprConstant.cpp:7329 + if (Result.isLValue()) + return Success(Result, E); + } ---------------- wchilders wrote: > This doesn't seem to be the right answer, and `ConstantExpr`s don't have > `LValue` `APValue`s, at least, not that are reaching this case. We had a > previous implementation that also, kind of punted on this issue with an > override in `TemporaryExprEvaluator`: > https://gitlab.com/lock3/clang/-/blob/9fbaeea06fc567ac472264bec2a72661a1e06c73/clang/lib/AST/ExprConstant.cpp#L9753 The base class version seems appropriate to me, even for this case. We eventually want to use `ConstantExpr` to store the evaluated initializer value of a `constexpr` variable (replacing the existing special-case caching on `VarDecl`) and the like, not only for immediate invocations, and once we start doing that for reference variables we'll have glvalue `ConstantExpr`s. Is there some circumstance under which a glvalue `ConstantExpr`'s `APValue` result is not of kind `LValue`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76438/new/ https://reviews.llvm.org/D76438 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
