nickdesaulniers added a comment.

In D151587#4519700 <https://reviews.llvm.org/D151587#4519700>, @efriedma wrote:

>> It seems like perhaps we want to call EvaluateAsLValue in 
>> Expr::isConstantInitializer AFTER checking the statement class. Otherwise 
>> we're basically reimplementing that big switch statement again; at least two 
>> cases as suggested by your diff.
>
> I think we want to keep lvalue and rvalue handling separate, for the same 
> reasons we want to keep it separate in other places.  It's very easy to get 
> the semantics wrong if you try to handle lvalues and rvalues in the same 
> switch.

Switching the order of operations seems to work with a slight fix for the 
`CastExprs`.  Let me post the diff and see what you think. Tests are green 
(FWIW). (I think that's cleaner than rechecking the Expr subclass and doing the 
same thing, but only for 2 cases).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151587

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to