[PATCH] D149834: [clang][Interp] Fix ignoring TypeTraitExprs

Timm Bäder via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 8 06:06:16 PDT 2023


tbaeder added inline comments.


================
Comment at: clang/test/AST/Interp/literals.cpp:875
     1 ? 0 : 1;
+    sizeof(A);
+    alignof(A);
----------------
tbaeder wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > Let's make sure we still reject this:
> > > > > ```
> > > > > constexpr int oh_my() {
> > > > >   int x = 0;
> > > > >   sizeof(int[x++]); // This would usually be evaluated because VLAs are terrible
> > > > >   return x;
> > > > > }
> > > > > ```
> > > > > https://godbolt.org/z/E3jx6co46
> > > > Hm, no, doesn't get rejected:
> > > > ```
> > > > array.cpp:1240:3: warning: expression result unused [-Wunused-value]
> > > >  1240 |   sizeof(int[x++]); // This would usually be evaluated because VLAs are terrible
> > > >       |   ^~~~~~~~~~~~~~~~
> > > > array.cpp:1243:15: error: static assertion failed due to requirement 'oh_my() == 1'
> > > >  1243 | static_assert(oh_my() == 1);
> > > >       |               ^~~~~~~~~~~~
> > > > array.cpp:1243:23: note: expression evaluates to '0 == 1'
> > > >  1243 | static_assert(oh_my() == 1);
> > > >       |               ~~~~~~~~^~~~
> > > > ```
> > > It looks to me like we're not modelling the side effects properly (we would return `1` if we were, so the `static_assert` would have passed), which might explain why we're failing to reject the code.
> > Right, I understand, I was just showing the current state. From looking at it though, this is not something to be fixed here, the `x++` is passed stand-alone to `evaluateAsRValue()`. Not sure yet what the exact problem is, I guess this is called before we even registered a local variable for `x`. I'll look into it.
> Fun, the `ArgumentExpr` is just `x++`:
> ```
>   |-UnaryExprOrTypeTraitExpr 0x621000072430 <line:1280:3, col:18> 'unsigned long' sizeof 'int[x++]'
>   | `-UnaryOperator 0x621000072378 <col:14, col:15> 'int' postfix '++'
>   |   `-DeclRefExpr 0x621000072350 <col:14> 'int' lvalue Var 0x6210000721a0 'x' 'int'
> ```
> it's simple to implement just rejecting it by just doing `discard(ArgumentExpr)`, but that doesn't emit any diagnostics.
... and now I just realized that the tests for this patch don't even fit with the changes themselves. `TypeTraitExpr` is not for `sizeof` and `alignof`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149834



More information about the cfe-commits mailing list