efriedma 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;
----------------
nickdesaulniers wrote:
> 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!!
Shouldn't that just be the following?
```
if (E->getStorageDuration() == SD_Static &&
Info.EvalMode == EvalInfo::EM_ConstantFold)
return false;
```
A materialized temporary is always going to be either an LValue or an XValue,
and the difference between the two isn't relevant here.
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
