tbaeder marked an inline comment as done. tbaeder added inline comments.
================ Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 + + if (!ArgType->isConstantSizeType() || + ArgType->isDependentType()) ---------------- erichkeane wrote: > Documentation for `isConstantSizedType` says it isn't legal to call it on > dependent or incomplete types. so this probably needs to be: > > ``` if (ArgType->isDependentType() || ArgType->isIncompleteType() || > !ArgType->isConstantSizedType())``` > > In roughly that order. `isDependentType()` is implicitly assumed, isn't it? `ExprConstant.cpp` asserts everywhere that neither types nor expressions are (value)dependent. It also doesn't handle that in `HandleSizeof`. ================ Comment at: clang/test/AST/Interp/literals.cpp:89 + static_assert(sizeof(&soint) == sizeof(void*), ""); + static_assert(sizeof(&soint) == sizeof(nullptr), ""); + ---------------- aaron.ballman wrote: > 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.) I'll add it, but might have to `#if 0` it out for the same reason I had to push https://reviews.llvm.org/rGa8843643cd75d0e93ebcf3f30b470d2b8e59868d 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