wchilders marked an inline comment as done. wchilders added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:7329 + if (Result.isLValue()) + return Success(Result, E); + } ---------------- rsmith wrote: > 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`? > Is there some circumstance under which a glvalue ConstantExpr's APValue > result is not of kind LValue? Good question, the `Sema/switch.c` test fails if you `assert(Result.isLValue())`. ``` ConstantExpr 0x555562ab8760 'const int' lvalue Int: 3 `-DeclRefExpr 0x555562ab8740 'const int' lvalue Var 0x555562ab8230 'a' 'const int' ``` With an attributed line no. 359: https://github.com/llvm/llvm-project/blob/4b0029995853fe37d1dc95ef96f46697c743fcad/clang/test/Sema/switch.c#L359 The offending RValue is created in SemaExpr.cpp here: https://github.com/llvm/llvm-project/blob/f87563661d6661fd4843beb8f391acd077b08dbe/clang/lib/Sema/SemaExpr.cpp#L15190 The issue stems from evaluating this as an RValue to produce an integer, then caching that RValue in an lvalue constant expression. Do you have any suggestions? Perhaps an LValue to RValue conversion should be performed on the expression if it folds correctly, so that the ConstantExpr is actually an RValue? 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
