aaron.ballman added a comment. In D133934#3794389 <https://reviews.llvm.org/D133934#3794389>, @tbaeder wrote:
> I can see that `HandleSizeOf()` uses 1 for void and function types as a gcc > extension, but I can't reproduce that: https://godbolt.org/z/njG9zh6PM C code allows the construct: https://godbolt.org/z/5KfY9PPqa Worth keeping in mind: C2x has support for `constexpr` objects, so we are going to have to care about this. We've not started doing the work to enable `constexpr` for C yet, so there's some wiggle room in terms of *when* we have to care, but we should try to keep C in mind when working on the new interpreter as much as is reasonable. ================ Comment at: clang/lib/AST/Interp/Boolean.h:50 explicit operator unsigned() const { return V; } + explicit operator int8_t() const { return V; } ---------------- At some point, do we want to rename this (and int) to use uint32_t/int32_t? ================ Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + return this->emitConst( + E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } ---------------- tbaeder wrote: > shafik wrote: > > aaron.ballman wrote: > > > erichkeane wrote: > > > > shafik wrote: > > > > > I notice that `HandleSizeof` special cases `void` and function types. > > > > OOOH, DEFINITELY make sure you test function types here, and figure out > > > > how HandleSizeof does it. > > > > > > > > BUT, I think 'void' should error/be an invalid before we get here? > > > > > > > > ALSO, now that I think further, I'm sure @aaron.ballman has a bunch of > > > > runtime-evaluated sizeof examples to share around vlas. > > > I was just about to comment about everyone's favorite horrible C++ > > > extension. :-D We should make sure we properly reject code like: > > > ``` > > > void func() { > > > int n = 12; > > > constexpr int oofda = sizeof(int[n++]); > > > } > > > ``` > > > while accepting code like: > > > ``` > > > consteval int foo(int n) { > > > return sizeof(int[n]); > > > } > > > constinit int var = foo(5); > > > ``` > > Note, that clang currently treats that as ill-formed and there is > > divergence with how gcc and clang treat the `constexpr` case as well. > > > > So if we are going to say we want this to work then we should file a bug > > against clang. > Right, the second example doesn't seem to be accepted by current clang. I'd add both of the test cases, put a FIXME comment above the one that currently fails but we'd like to see succeed. We can work on fixing that fixme whenever we get around to it. ================ Comment at: clang/test/AST/Interp/literals.cpp:89 + static_assert(sizeof(&soint) == sizeof(void*), ""); + static_assert(sizeof(&soint) == sizeof(nullptr), ""); + ---------------- This test got me thinking about another test case: ``` struct S { void func(); }; constexpr void (S::*Func)() = &S::func; static_assert(sizeof(Func) == sizeof(&S::func), ""); ``` (Can't test this one against `nullptr` because `sizeof(nullptr)` is not telling you the size of a pointer to member function despite `nullptr` being convertible to one.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133934/new/ https://reviews.llvm.org/D133934 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
