rsmith added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:7329 + if (Result.isLValue()) + return Success(Result, E); + } ---------------- wchilders wrote: > 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? I think we should be inserting the lvalue-to-rvalue conversion before we even try evaluating the expression. Does this go wrong along both the C++11 and pre-C++11 codepaths? (They do rather different conversions to the expression.) In any case, we're likely missing a call to `DefaultLvalueConversion` on whichever side is failing. Judging by the fact that `struct X { void f() { switch (0) case f: ; } };` misses the "non-static member function must be called" diagnostic only in C++98 mode, I'd imagine it's just the pre-C++11 codepath that's broken here. 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
