[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-09-05 Thread Timm Bäder via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d79f985b53e: [clang][Interp] Implement zero-init of record 
types (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D154189?vs=545367=555836#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -911,3 +911,115 @@
   }
   static_assert(foo(F()) == 0, "");
 }
+
+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(x.b) + x.c[0] + x.c[3] + static_cast(x.d);
+}
+static_assert(foo(A()) == 0, "");
+  }
+
+  namespace Inheritance {
+struct F2 : F {
+  float f;
+};
+
+constexpr int foo(F2 f) {
+  return (int)f.f + f.a;
+}
+static_assert(foo(F2()) == 0, "");
+  }
+
+  namespace BitFields {
+struct F {
+  unsigned a : 6;
+};
+constexpr int foo(F f) {
+  return f.a;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace Nested {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2.f + f.f2.c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace CompositeArrays {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2[2];
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2[0].f + f.f2[0].c + f.f2[1].f + f.f2[1].c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  /// FIXME: This needs support for unions on the new interpreter.
+  /// We diagnose an uninitialized object in c++14.
+#if __cplusplus > 201402L
+  namespace Unions {
+struct F {
+  union {
+int a;
+char c[4];
+float f;
+  } U;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.U.f; // ref-note {{read of member 'f' of union with active member 'a'}}
+}
+static_assert(foo(F()) == 0, ""); // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
+  }
+#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 uninitialized object}} \
+  // 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
+}
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -138,6 +138,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -285,6 +285,12 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -213,6 +213,7 @@
 
   /// 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.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1302,7 

[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-31 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 545367.
tbaeder marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -931,3 +931,116 @@
   O o1(0);
 }
 #endif
+
+
+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(x.b) + x.c[0] + x.c[3] + static_cast(x.d);
+}
+static_assert(foo(A()) == 0, "");
+  }
+
+  namespace Inheritance {
+struct F2 : F {
+  float f;
+};
+
+constexpr int foo(F2 f) {
+  return (int)f.f + f.a;
+}
+static_assert(foo(F2()) == 0, "");
+  }
+
+  namespace BitFields {
+struct F {
+  unsigned a : 6;
+};
+constexpr int foo(F f) {
+  return f.a;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace Nested {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2.f + f.f2.c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace CompositeArrays {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2[2];
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2[0].f + f.f2[0].c + f.f2[1].f + f.f2[1].c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  /// FIXME: This needs support for unions on the new interpreter.
+  /// We diagnose an uninitialized object in c++14.
+#if __cplusplus > 201402L
+  namespace Unions {
+struct F {
+  union {
+int a;
+char c[4];
+float f;
+  } U;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.U.f; // ref-note {{read of member 'f' of union with active member 'a'}}
+}
+static_assert(foo(F()) == 0, ""); // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
+  }
+#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 uninitialized object}} \
+  // 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
+}
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -140,6 +140,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -289,6 +289,12 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -222,6 +222,7 @@
 
   /// 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.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1394,7 +1394,15 @@
   assert(!classify(T));
 
   if (T->isRecordType()) {
-const Function *Func = getFunction(E->getConstructor());
+const CXXConstructorDecl *Ctor = E->getConstructor();
+
+// Trivial zero initialization.
+if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+   

[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240
+  // Fields
+  for (const Record::Field  : R->fields()) {
+const Descriptor *D = Field.Desc;

tbaeder wrote:
> aaron.ballman wrote:
> > 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.
> 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.
> 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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 545146.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -931,3 +931,63 @@
   O o1(0);
 }
 #endif
+
+
+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(x.b) + x.c[0] + x.c[3] + static_cast(x.d);
+}
+static_assert(foo(A()) == 0, "");
+  }
+
+  namespace Inheritance {
+struct F2 : F {
+  float f;
+};
+
+constexpr int foo(F2 f) {
+  return (int)f.f + f.a;
+}
+static_assert(foo(F2()) == 0, "");
+  }
+
+  namespace BitFields {
+struct F {
+  unsigned a : 6;
+};
+constexpr int foo(F f) {
+  return f.a;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+
+#if __cplusplus >= 202002L
+  namespace Failure {
+struct S {
+  int a;
+  F f{12};
+};
+constexpr int foo(S x) {
+  return x.a; // expected-note {{read of uninitialized object}} \
+  // 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
+}
+
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -140,6 +140,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -289,6 +289,12 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -222,6 +222,7 @@
 
   /// 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.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1394,7 +1394,15 @@
   assert(!classify(T));
 
   if (T->isRecordType()) {
-const Function *Func = getFunction(E->getConstructor());
+const CXXConstructorDecl *Ctor = E->getConstructor();
+
+// Trivial zero initialization.
+if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+  const Record *R = getRecord(E->getType());
+  return this->visitZeroRecordInitializer(R, E);
+}
+
+const Function *Func = getFunction(Ctor);
 
 if (!Func)
   return false;
@@ -1698,6 +1706,62 @@
   llvm_unreachable("unknown primitive type");
 }
 
+template 
+bool ByteCodeExprGen::visitZeroRecordInitializer(const Record *R,
+  const Expr *E) {
+  assert(E);
+  assert(R);
+  // Fields
+  for (const Record::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 

[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:939
+  };
+#endif
+

aaron.ballman wrote:
> 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)();
> };
> ```
member pointers are also not handled yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240
+  // Fields
+  for (const Record::Field  : R->fields()) {
+const Descriptor *D = Field.Desc;

aaron.ballman wrote:
> 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.
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.



Comment at: clang/lib/AST/Interp/Descriptor.cpp:281-289
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  QualType T = getType();
+
+  const auto *CAT = cast(T);
+  return CAT->getElementType();
+

aaron.ballman wrote:
> 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?
I only need an `ArrayType` anyway, so should be fine.



Comment at: clang/test/AST/Interp/records.cpp:931
+constexpr int foo(S x) {
+  return x.a; // expected-note {{read of object outside its lifetime}} \
+  // ref-note {{read of uninitialized object}}

aaron.ballman wrote:
> Why is `x.a` outside of its lifetime?
That was just a wrong diagnostic that has since been fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 545131.
tbaeder marked 2 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -931,3 +931,63 @@
   O o1(0);
 }
 #endif
+
+
+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(x.b) + x.c[0] + x.c[3] + static_cast(x.d);
+}
+static_assert(foo(A()) == 0, "");
+  }
+
+  namespace Inheritance {
+struct F2 : F {
+  float f;
+};
+
+constexpr int foo(F2 f) {
+  return (int)f.f + f.a;
+}
+static_assert(foo(F2()) == 0, "");
+  }
+
+  namespace BitFields {
+struct F {
+  unsigned a : 6;
+};
+constexpr int foo(F f) {
+  return f.a;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+
+#if __cplusplus >= 202002L
+  namespace Failure {
+struct S {
+  int a;
+  F f{12};
+};
+constexpr int foo(S x) {
+  return x.a; // expected-note {{read of uninitialized object}} \
+  // 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
+}
+
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -140,6 +140,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -289,6 +289,12 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -222,6 +222,7 @@
 
   /// 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.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1394,7 +1394,15 @@
   assert(!classify(T));
 
   if (T->isRecordType()) {
-const Function *Func = getFunction(E->getConstructor());
+const CXXConstructorDecl *Ctor = E->getConstructor();
+
+// Trivial zero initialization.
+if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+  const Record *R = getRecord(E->getType());
+  return this->visitZeroRecordInitializer(R, E);
+}
+
+const Function *Func = getFunction(Ctor);
 
 if (!Func)
   return false;
@@ -1467,7 +1475,7 @@
 return true;
   }
 
-  return false;
+  return DiscardResult ? this->emitPopPtr(E) : true;
 }
 
 template 
@@ -1698,6 +1706,62 @@
   llvm_unreachable("unknown primitive type");
 }
 
+template 
+bool ByteCodeExprGen::visitZeroRecordInitializer(const Record *R,
+  const Expr *E) {
+  assert(E);
+  assert(R);
+  // Fields
+  for (const Record::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 

[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240
+  // Fields
+  for (const Record::Field  : R->fields()) {
+const Descriptor *D = Field.Desc;

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.



Comment at: clang/lib/AST/Interp/Descriptor.cpp:281-289
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  QualType T = getType();
+
+  const auto *CAT = cast(T);
+  return CAT->getElementType();
+

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?



Comment at: clang/test/AST/Interp/records.cpp:931
+constexpr int foo(S x) {
+  return x.a; // expected-note {{read of object outside its lifetime}} \
+  // ref-note {{read of uninitialized object}}

Why is `x.a` outside of its lifetime?



Comment at: clang/test/AST/Interp/records.cpp:939
+  };
+#endif
+

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)();
};
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-18 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-07-06 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-06-30 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1115
+  // all field and bases explicitly.
+  if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+assert(E->getType()->isRecordType());

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154189/new/

https://reviews.llvm.org/D154189

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-06-30 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154189

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -899,3 +899,45 @@
   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(x.b) + x.c[0] + x.c[3] + static_cast(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
+
+
+}
+
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -137,6 +137,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -278,6 +278,16 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  QualType T = getType();
+
+  const auto *CAT = cast(T);
+  return CAT->getElementType();
+
+  return T;
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -92,7 +92,6 @@
   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);
@@ -210,6 +209,7 @@
 
   /// 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.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1019,24 +1019,6 @@
   return this->visit(E->getSubExpr());
 }
 
-template 
-bool ByteCodeExprGen::VisitCXXTemporaryObjectExpr(
-const CXXTemporaryObjectExpr *E) {
-  if (std::optional 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 
 bool ByteCodeExprGen::VisitCompoundLiteralExpr(
 const CompoundLiteralExpr *E) {
@@ -1119,19 +1101,30 @@
 template 
 bool ByteCodeExprGen::VisitCXXConstructExpr(
 const CXXConstructExpr *E) {
-  if (std::optional GI = allocateLocal(E, /*IsExtended=*/false)) {
-if (!this->emitGetPtrLocal(*GI, E))
-  return false;
+  std::optional