[clang][Interp] Implement zero-init of record types

[tbaeder]

Summary

Unify CXXTemporaryObjectExpr and CXXConstructExpr code and zero-initialize the object if requested.

Hints on how to test this properly without creating some weird intermediate object are welcome of course. :)

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1115

I briefly talked about this with @aaron.ballman on IRC. The current interpreter properly zero-initialized all the fields of a struct, but the un-does the initialization again (and initializes them to a APValue::Indeterminate in HandleConstructorCall::SkipToField().

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1240

It looks like you're not initializing base classes or padding bits: http://eel.is/c++draft/dcl.init#general-6.2 -- we could use test coverage for both of those cases (don't forget to also test bit-fields). You should also have a test for zero init of unions.

clang/lib/AST/Interp/Descriptor.cpp

281–289

This looks pretty dangerous -- is it safe to assume the type is a constant array, or should this be a dyn_cast and the spurious return is for the else branch?

clang/test/AST/Interp/records.cpp

931

Why is x.a outside of its lifetime?

939

I think we should have more test coverage. I mentioned a few above, but other things to test would be inner classes (and anonymous members), like:

struct S {
  struct {
    int x, y;
  } v;
  union {
    float a;
    int b;
  };
};

and member pointers (because those aren't simple pointers):

struct A {
  void func();
};
struct S {
  void (A::*ptr)();
};

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1240

Unions and bitfields are generally not supported yet, and I'm not sure what you mean by padding bits - they don't exist at this stage. In storage however, they are always zero since we memset that to 0.

clang/lib/AST/Interp/Descriptor.cpp

281–289

I only need an ArrayType anyway, so should be fine.

clang/test/AST/Interp/records.cpp

931

That was just a wrong diagnostic that has since been fixed.

[tbaeder]

clang/test/AST/Interp/records.cpp

939

member pointers are also not handled yet.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

1240

Unions and bitfields are generally not supported yet,

Let's add the test coverage and mark failures with FIXME comments; otherwise we risk forgetting to add the test coverage later.

and I'm not sure what you mean by padding bits - they don't exist at this stage. In storage however, they are always zero since we memset that to 0.

So long as we're validating that they're properly set to zero; that's the important part.

[aaron.ballman]

LGTM

clang/lib/AST/Interp/ByteCodeExprGen.h

public:
   bool VisitStringLiteral(const StringLiteral *E);
   bool VisitCharacterLiteral(const CharacterLiteral *E);
   bool VisitCompoundAssignOperator(const CompoundAssignOperator *E);
   bool VisitFloatCompoundAssignOperator(const CompoundAssignOperator *E);
   bool VisitPointerCompoundAssignOperator(const CompoundAssignOperator *E);
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
   bool VisitPredefinedExpr(const PredefinedExpr *E);
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
private:
   friend class DestructorScope<Emitter>;
   friend class RecordScope<Emitter>;
   friend class DeclScope<Emitter>;
   friend class OptionScope<Emitter>;
   friend class ArrayIndexScope<Emitter>;
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(QualType QT, const Expr *E);
+  bool visitZeroRecordInitializer(const Record *R, const Expr *E);
 
   enum class DerefKind {
     /// Value is read and pushed to stack.
     Read,
     /// Direct method generates a value which is written. Returns pointer.
     Write,
     /// Direct method receives the value, pushes mutated value. Returns pointer.
     ReadWrite,

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCXXBindTemporaryExpr(
     const CXXBindTemporaryExpr *E) {
 
   return this->visit(E->getSubExpr());
 }
 
 template <class Emitter>
-bool ByteCodeExprGen<Emitter>::VisitCXXTemporaryObjectExpr(
-    const CXXTemporaryObjectExpr *E) {
-  if (std::optional<unsigned> LocalIndex =
-          allocateLocal(E, /*IsExtended=*/false)) {
-    if (!this->emitGetPtrLocal(*LocalIndex, E))
-      return false;
-
-    if (!this->visitInitializer(E))
-      return false;
-
-    if (DiscardResult)
-      return this->emitPopPtr(E);
-    return true;
-  }
-  return false;
-}
-
-template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCompoundLiteralExpr(
     const CompoundLiteralExpr *E) {
   std::optional<PrimType> T = classify(E->getType());
   const Expr *Init = E->getInitializer();
   if (E->isFileScope()) {
     if (std::optional<unsigned> GlobalIndex = P.createGlobal(E)) {
       if (classify(E->getType()))
         return this->visit(Init);
if (!this->discard(E->getSubExpr()))
     return false;
 
   return this->emitInvalidCast(CastKind::Reinterpret, E);
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
     const CXXConstructExpr *E) {
-  if (std::optional<unsigned> GI = allocateLocal(E, /*IsExtended=*/false)) {
-    if (!this->emitGetPtrLocal(*GI, E))
-      return false;
+  std::optional<unsigned> LocalIndex = allocateLocal(E, /*IsExtended=*/false);
+  if (!LocalIndex)
 
-    if (!this->visitRecordInitializer(E))
-      return false;
+  if (!this->emitGetPtrLocal(*LocalIndex, E))
 
-    if (DiscardResult)
-      return this->emitPopPtr(E);
-    return true;
-  }
+  const CXXConstructorDecl *Ctor = E->getConstructor();
 
+  // If zero-init is requested and the constructor is trivial, zero-init
+  // all field and bases explicitly.
+  if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+    assert(E->getType()->isRecordType());
+    const Record *R = getRecord(E->getType());
+
+    if (!this->visitZeroRecordInitializer(R, E))
+      return false;
+  } else {
+    // Otherwise, just call the constructor.
+    if (!this->visitInitializer(E))
       return false;
   }
 
+  return DiscardResult ? this->emitPopPtr(E) : true;
+}
+
 template <class Emitter> bool ByteCodeExprGen<Emitter>::discard(const Expr *E) {
   if (E->containsErrors())
     return false;
 
   OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
 
bool ByteCodeExprGen<Emitter>::visitZeroInitializer(QualType QT,
   case PT_FnPtr:
     return this->emitNullFnPtr(E);
   case PT_Float: {
     return this->emitConstFloat(APFloat::getZero(Ctx.getFloatSemantics(QT)), E);
   }
   }
   llvm_unreachable("unknown primitive type");
 }
+template <class Emitter>
+bool ByteCodeExprGen<Emitter>::visitZeroRecordInitializer(const Record *R,
+                                                          const Expr *E) {
+  // Fields
+  for (const Record::Field &Field : R->fields()) {
+    const Descriptor *D = Field.Desc;
+    if (D->isPrimitive()) {
+      QualType QT = D->getType();
+      PrimType T = classifyPrim(D->getType());
+      if (!this->visitZeroInitializer(QT, E))
+        return false;
+      if (!this->emitInitField(T, Field.Offset, E))
+        return false;
+      continue;
+    }
+
+    // TODO: Add GetPtrFieldPop and get rid of this dup.
+    if (!this->emitDupPtr(E))
+      return false;
+    if (!this->emitGetPtrField(Field.Offset, E))
+      return false;
+
+    if (D->isPrimitiveArray()) {
+      QualType ET = D->getElemQualType();
+      PrimType T = classifyPrim(ET);
+      for (uint32_t I = 0, N = D->getNumElems(); I != N; ++I) {
+        if (!this->visitZeroInitializer(ET, E))
+          return false;
+        if (!this->emitInitElem(T, I, E))
+          return false;
+      }
+    } else {
+      // FIXME: Implement class and composite array types.
+      assert(false);
+    }
+
+    if (!this->emitPopPtr(E))
+      return false;
+  }
+
+  return true;
+}
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::dereference(
     const Expr *LV, DerefKind AK, llvm::function_ref<bool(PrimType)> Direct,
     llvm::function_ref<bool(PrimType)> Indirect) {
   if (std::optional<PrimType> T = classify(LV->getType())) {
     if (!LV->refersToBitField()) {
       // Only primitive, non bit-field types can be dereferenced directly.

clang/lib/AST/Interp/Descriptor.h

public:
   /// Allocates a descriptor for an array of composites of unknown size.
   Descriptor(const DeclTy &D, Descriptor *Elem, bool IsTemporary, UnknownSize);
 
   /// Allocates a descriptor for a record.
   Descriptor(const DeclTy &D, Record *R, MetadataSize MD, bool IsConst,
              bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast<const Decl *>(); }
   const Expr *asExpr() const { return Source.dyn_cast<const Expr *>(); }
 
   const ValueDecl *asValueDecl() const {
     return dyn_cast_if_present<ValueDecl>(asDecl());
   }

clang/lib/AST/Interp/Descriptor.cpp

if (auto *E = asExpr())
     return E->getType();
   if (auto *D = asValueDecl())
     return D->getType();
   if (auto *T = dyn_cast<TypeDecl>(asDecl()))
     return QualType(T->getTypeForDecl(), 0);
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  QualType T = getType();
+
+  const auto *CAT = cast<ConstantArrayType>(T);
+  return CAT->getElementType();
+
+  return T;
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast<const Decl *>())
     return D->getLocation();
   if (auto *E = Source.dyn_cast<const Expr *>())
     return E->getExprLoc();
   llvm_unreachable("Invalid descriptor type");
 }
 

clang/test/AST/Interp/records.cpp

 #endif
 
 }
 
 constexpr bool BPand(BoolPair bp) {
   return bp.first && bp.second;
 }
 static_assert(BPand(BoolPair{true, false}) == false, "");
+
+
+namespace ZeroInit {
+  struct F {
+    int a;
+  };
+
+  namespace Simple {
+    struct A {
+      char a;
+      bool b;
+      int c[4];
+      float d;
+    };
+    constexpr int foo(A x) {
+      return x.a + static_cast<int>(x.b) + x.c[0] + x.c[3] + static_cast<int>(x.d);
+    }
+#if __cplusplus >= 201703L
+    static_assert(foo(A()) == 0, "");
+#endif
+  }
+
+#if __cplusplus >= 202002L
+  namespace Failure {
+    struct S {
+      int a;
+      F f{12};
+    };
+    constexpr int foo(S x) {
+      return x.a; // expected-note {{read of object outside its lifetime}} \
+                  // ref-note {{read of uninitialized object}}
+    }
+    static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \
+                                      // expected-note {{in call to}} \
+                                      // ref-error {{not an integral constant expression}} \
+                                      // ref-note {{in call to}}
+  };
+#endif
+
+
+}
+