nickdesaulniers added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:8360
+ // Do not constant fold an R-value.
+ if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue())
+ return false;
----------------
efriedma wrote:
> Checking isLValue() doesn't make sense; consider:
>
> ```
> struct R { mutable long x; };
> struct Z { const R &x, y; };
> Z z = { R{1}, z.x.x=10 };
> ```
>
> Maybe also want to check for EM_IgnoreSideEffects? Not sure what cases, if
> any, that would affect.
>
> We should probably check `E->getStorageDuration() == SD_Static`. The cases
> where it's a local temporary don't hit the getOrCreateValue() codepath, so
> the evaluated value should be handled correctly.
>
> Checking EvalMode feels a little weird, but I guess it should do the right
> thing in the cases I can think of? I'd like a second opinion on this.
Changing this condition to:
```
if (E->getStorageDuration() == SD_Static &&
Info.EvalMode == EvalInfo::EM_ConstantFold &&
E->isXValue())
return false;
```
allows all tests in tree to pass, but messes up the test case you posted above.
I'm trying to sus out what else might be different about that test case...we
should return `false` for that, but I'm not sure what's different about that
case.
In particular, I was playing with `E->isUsableInConstantExpressions` and
`E->getLifetimeExtendedTemporaryDecl()`, but getting your case to work, I end
up regressing
clang/test/SemaCXX/attr-require-constant-initialization.cpp...argh!!
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
