keryell added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3704-3705
+ Result = E->EvaluateAsRValue(Eval, Context, true);
+ else
+ Result = E->EvaluateAsLValue(Eval, Context, true);
+
----------------
Tyker wrote:
> keryell wrote:
> > aaron.ballman wrote:
> > > Tyker wrote:
> > > > aaron.ballman wrote:
> > > > > Under what circumstances would we want the constant expressions to be
> > > > > lvalues? I would have guessed you would want to call
> > > > > `Expr::EvaluateAsConstantExpr()` instead of either of these.
> > > > Expr::EvaluateAsConstantExpr will evaluate expression in there value
> > > > category.
> > > > this can be quite surprising in some situations like:
> > > > ```
> > > > const int g_i = 0;
> > > > [[clang::annotate("test", g_i)]] void t() {} // annotation carries a
> > > > pointer/reference on g_i
> > > > [[clang::annotate("test", (int)g_i)]] void t1() {} // annotation
> > > > carries the value 0
> > > > [[clang::annotate("test", g_i + 0)]] void t1() {} // annotation carries
> > > > the value 0
> > > >
> > > > ```
> > > > with EvaluateAsRValue in all of the cases above the annotation will
> > > > carry the value 0.
> > > >
> > > > optionally we could wrap expression with an LValue to RValue cast and
> > > > call EvaluateAsConstantExpr this should also work.
> > > Thank you for the explanation. I think wrapping with an lvalue to rvalue
> > > conversion may make more sense -- `EvaluateAsRValue()` tries really hard
> > > to get an rvalue out of something even if the standard says that's not
> > > okay. It'll automatically apply the lvalue to rvalue conversion if
> > > appropriate, but it'll also do more than that (such as evaluating side
> > > effects).
> > This needs some motivating comments and explanations in the code.
> > Also the variations on `g_i` annotation above are quite interesting and
> > should appear in the unit tests. I am unsure they are now.
> > Perhaps more interesting with other values than `0` in the example. :-)
> > I guess we can still use `[[clang::annotate("test", &g_i)]] void t() {}` to
> > have a pointer on `g_i` and `(int&)g_i` for reference? To add to the unit
> > tests if not already checked.
> >
> i changed to use EvaluateAsConstantExpr + LValueToRvalue cast.
>
> > This needs some motivating comments and explanations in the code.
> > Also the variations on `g_i` annotation above are quite interesting and
> > should appear in the unit tests. I am unsure they are now.
> they are tested not as expressively as above but they are tested.
>
> > Perhaps more interesting with other values than `0` in the example. :-)
> > I guess we can still use `[[clang::annotate("test", &g_i)]] void t() {}` to
> > have a pointer on `g_i`
> yes this is how it is working.
> > `(int&)g_i` for reference? To add to the unit tests if not already checked.
> (int&)g_i has the salve value category and same type as g_i it just has a
> noop cast around it.
> pointer an reference are indistinguishable in an APValue the only difference
> is the c++ type of what they represent.
> and they are of course lowered to the same thing. so we only need the pointer
> case.
>
I see. Thanks for the clarification.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88645/new/
https://reviews.llvm.org/D88645
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
