tbaeder added inline comments.
================ Comment at: clang/test/AST/Interp/literals.cpp:875 1 ? 0 : 1; + sizeof(A); + alignof(A); ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149834/new/ https://reviews.llvm.org/D149834 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits