[clang][Interp] Handle sizeof() expressions

[tbaeder]

Summary

Handle sizeof expressions.

I was a bit unsure here about alignment, but the rest is pretty self-explanatory I think.

[erichkeane]

clang/lib/AST/Interp/Context.cpp

134 ↗

(On Diff #460355)

This function seems pretty large for something you should be able to pick up via ASTContext::getTypeSize or ASTContext::getTypeSizeInChars or ASTContext::getTypeSizeInCharsIfKnown.

149 ↗

(On Diff #460355)

Typically, I prefer we use llvm::None here.

[tbaeder]

Didn't now I could just get that information from the ASTContext, nice. Thanks.

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

289

You probably want getTypeSizeInChars. getTypeSize returns it in bits, so this would give the wrong answer. You should probably add some tests to make sure the values you are getting back are sane (like char is 1, etc).

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

289

Yeah, right. I explicitly didn't compare with fixed values when writing the tests so I don't run into the usual problems, but they wouldn't caught this problem...

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

I notice that HandleSizeof special cases void and function types.

clang/test/AST/Interp/literals.cpp

83

Test for void and a function type too.

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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);

[shafik]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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.

[tbaeder]

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

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

Right, the second example doesn't seem to be accepted by current clang.

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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.

[aaron.ballman]

In 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.

clang/lib/AST/Interp/Boolean.h

50–54

At some point, do we want to rename this (and int) to use uint32_t/int32_t?

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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.

clang/test/AST/Interp/literals.cpp

89

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.)

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

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.

clang/test/AST/Interp/literals.cpp

89

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

[erichkeane]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

Ok, good to know about that then. I saw you'd tested for isDependentType above, but isConstantSizedType would assert in that case.

clang/test/AST/Interp/literals.cpp

89

What is the cause of that leak? Is that something we should be fixing ASAP so we can enable these tests?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

Yup, I switched the order around.

clang/test/AST/Interp/literals.cpp

89

IIRC it was creating a Record for the RecordDecl but never freeing it or something similar. The test case didn't do anything anyway since records aren't implemented yet. The only difference now is that I won't notice automatically that a test case suddenly starts working when I implement records.

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

290

... and of course I run into an incomplete type assertion with the sizeof(void) example when the file type is C.

[tbaeder]

clang/lib/AST/Interp/Boolean.h

50–54

I can do that as a follow-up commit if you want.

[erichkeane]

clang/lib/AST/Interp/Boolean.h

50–54

Please do, while we don't currently compile on any platforms with non-32 bit int/unsigned, if we ever tried to this would fail with multiple definitions somewhere.

[tbaeder]

@erichkeane Anything else still missing here?

[erichkeane]

I think we're OK for now, LGTM.

clang/lib/AST/Interp/Boolean.h

public:
   bool operator!=(Boolean RHS) const { return V != RHS.V; }
 
   bool operator>(unsigned RHS) const { return static_cast<unsigned>(V) > RHS; }
   bool operator>=(unsigned RHS) const { return static_cast<unsigned>(V) >= RHS; }
 
   Boolean operator-() const { return Boolean(V); }
   Boolean operator~() const { return Boolean(true); }
 
   explicit operator unsigned() const { return V; }
+  explicit operator int8_t() const { return V; }
+  explicit operator uint8_t() const { return V; }
+  explicit operator int16_t() const { return V; }
+  explicit operator uint16_t() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
   explicit operator int() const { return V; }
   explicit operator bool() const { return V; }
 
   APSInt toAPSInt() const {
     return APSInt(APInt(1, static_cast<uint64_t>(V), false), true);
   }

clang/lib/AST/Interp/ByteCodeExprGen.h

public:
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
   bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E);
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
+  bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
   bool visitDecl(const VarDecl *VD) override;
 
 protected:
   /// Emits scope cleanup instructions.
   void emitCleanup();

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitConstantExpr(const ConstantExpr *E) {
   // TODO: Check if the ConstantExpr already has a value set and if so,
   //   use that instead of evaluating it again.
   return this->visit(E->getSubExpr());
 }
 
 template <class Emitter>
-bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
+bool ByteCodeExprGen<Emitter>::VisitUnaryExprOrTypeTraitExpr(
+    const UnaryExprOrTypeTraitExpr *E) {
+
+  if (E->getKind() == UETT_SizeOf) {
+    QualType ArgType = E->getTypeOfArgument();
+    return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType));
+  }
+
+  return false;
+}
+
+template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visit(const Expr *E) {
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/false);
   return this->Visit(E);

clang/lib/AST/Interp/Opcodes.td

 }
 
 //===----------------------------------------------------------------------===//
 // Cast.
 //===----------------------------------------------------------------------===//
 // TODO: Expand this to handle casts between more types.
 
 def FromCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def ToCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def Cast: Opcode {
   let Types = [FromCastTypeClass, ToCastTypeClass];
   let HasGroup = 1;
 }
 
 //===----------------------------------------------------------------------===//

clang/test/AST/Interp/literals.cpp

 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
 static_assert(*p == 10, "");
 
 constexpr const int* getIntPointer() {
   return &m;
 }
-//static_assert(getIntPointer() == &m, ""); TODO
-//static_assert(*getIntPointer() == 10, ""); TODO
+static_assert(getIntPointer() == &m, "");
+static_assert(*getIntPointer() == 10, "");
 
 constexpr int gimme(int k) {
   return k;
 }
-// static_assert(gimme(5) == 5, ""); TODO
+static_assert(gimme(5) == 5, "");
+
+namespace SizeOf {
+  constexpr int soint = sizeof(int);
+  constexpr int souint = sizeof(unsigned int);
+  static_assert(soint == souint, "");
+
+  static_assert(sizeof(&soint) == sizeof(void*), "");
+  static_assert(sizeof(&soint) == sizeof(nullptr), "");
+
+  static_assert(sizeof(long) == sizeof(unsigned long), "");
+  static_assert(sizeof(char) == sizeof(unsigned char), "");
+
+  constexpr int N = 4;
+  constexpr int arr[N] = {1,2,3,4};
+  static_assert(sizeof(arr) == N * sizeof(int), "");
+  static_assert(sizeof(arr) == N * sizeof(arr[0]), "");
+
+  constexpr bool arrB[N] = {true, true, true, true};
+  static_assert(sizeof(arrB) == N * sizeof(bool), "");
+};