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

Reply via email to