[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D132727#3779237 , @tbaeder wrote:

> Thanks @MaskRay, I thought this was fixed by 
> https://github.com/llvm/llvm-project/commit/86271798e51a7866dd2af44e0ee183d1331089e6,
>  at least all the emails I got about failing msan builds were missing that 
> change.

For a msan failure of `llvm/clang/test/AST/Interp/arrays.cpp`, I have verified 
that only after 49c289879c7aa9737429f7342c7149a8ba958667 
 it is 
fixed:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Thanks @MaskRay, I thought this was fixed by 
https://github.com/llvm/llvm-project/commit/86271798e51a7866dd2af44e0ee183d1331089e6,
 at least all the emails I got about failing msan builds were missing that 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Heads-up: after D132832  and D132727 
, clang/test/AST/Interp/arrays.cpp.test has a 
msan failure. It can be pre-existing or can be newly introduced.
There seems another issue about an integer overflow in `InitMap::isInitialized`.

Still investigating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[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 rG3a7d476087df: [clang][Interp] Implement array initializers 
and subscript expressions (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -1,13 +1,85 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
 
-/// expected-no-diagnostics
-/// ref-no-diagnostics
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wc99-extensions"
 #pragma clang diagnostic ignored "-Winitializer-overrides"
+constexpr int DI[] = {
+  [0] = 10,
+  [1] = 20,
+  30,
+  40,
+  [1] = 50
+};
+static_assert(DI[0] == 10, "");
+static_assert(DI[1] == 50, "");
+static_assert(DI[2] == 30, "");
+static_assert(DI[3] == 40, "");
+
 /// FIXME: The example below tests ImplicitValueInitExprs, but we can't
 ///   currently evaluate other parts of it.
 #if 0
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot in

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[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/D132727/new/

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 457767.
tbaeder marked 6 inline comments as done.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -1,13 +1,85 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
 
-/// expected-no-diagnostics
-/// ref-no-diagnostics
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wc99-extensions"
 #pragma clang diagnostic ignored "-Winitializer-overrides"
+constexpr int DI[] = {
+  [0] = 10,
+  [1] = 20,
+  30,
+  40,
+  [1] = 50
+};
+static_assert(DI[0] == 10, "");
+static_assert(DI[1] == 50, "");
+static_assert(DI[2] == 30, "");
+static_assert(DI[3] == 40, "");
+
 /// FIXME: The example below tests ImplicitValueInitExprs, but we can't
 ///   currently evaluate other parts of it.
 #if 0
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMa

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:277
+template 
+bool ByteCodeExprGen::VisitConstantExpr (const ConstantExpr *E) {
+  // TODO: Check if the ConstantExpr already has a value set and if so,





Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:246
 
-auto Off = this->allocateLocalPrimitive(VD, *T, DT.isConstQualified());
+auto Offset =
+this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());





Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:256
+return this->emitSetLocal(*T, Offset, VD);
   } else {
 // Composite types - allocate storage and initialize it.

You can drop this `else` because of the preceding unconditional `return`.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:258
 // Composite types - allocate storage and initialize it.
-if (auto Off = this->allocateLocal(VD)) {
-  return this->visitLocalInitializer(VD->getInit(), *Off);
-} else {
+if (auto Offset = this->allocateLocal(VD))
+  return this->visitLocalInitializer(VD->getInit(), *Offset);





Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:260
+  return this->visitLocalInitializer(VD->getInit(), *Offset);
+else
   return this->bail(VD);

You should drop this `else` as well.



Comment at: clang/lib/AST/Interp/Pointer.cpp:158
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {

`isArray()` already covers `isPrimitiveArray()` so this change looks 
unnecessary.


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 456953.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -1,13 +1,85 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
 
-/// expected-no-diagnostics
-/// ref-no-diagnostics
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wc99-extensions"
 #pragma clang diagnostic ignored "-Winitializer-overrides"
+constexpr int DI[] = {
+  [0] = 10,
+  [1] = 20,
+  30,
+  40,
+  [1] = 50
+};
+static_assert(DI[0] == 10, "");
+static_assert(DI[1] == 50, "");
+static_assert(DI[2] == 30, "");
+static_assert(DI[3] == 40, "");
+
 /// FIXME: The example below tests ImplicitValueInitExprs, but we can't
 ///   currently evaluate other parts of it.
 #if 0
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive arra

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder marked an inline comment as done.
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:41-44
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}

aaron.ballman wrote:
> A similar test we might want to add (whenever we get around to references):
> ```
> template 
> constexpr T& getElementOf(T (&array)[N], int I) {
>   return array[I];
> }
> static_assert(getElementOf(foo[2], 3) == &m, "");
> ```
I added that to https://reviews.llvm.org/D132997


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D132727#3755843 , @tbaeder wrote:

> In D132727#3755571 , @aaron.ballman 
> wrote:
>
>> Precommit CI found a relevant failure:
>
> That needs the lvalue-to-rvalue conversion patch first.

Ah, good to know!

FWIW, one concern I've been pondering is building the new constant expression 
interpreter in such a way that we're working on higher-level constructs before 
having the lower-level constructs supported. I worry we're going to miss test 
coverage because we won't be able to add it until some later changes come in 
and then we'll forget to go back and add it. There's nothing specific to this 
review (nor actionable here), but just something to keep in mind as you're 
working on stuff. Implementing things like the type system (lvalue references, 
rvalue references, etc) first and then building other layers on top of that 
(like function calls, variables, etc) when plausible would ease those concerns.




Comment at: clang/test/AST/Interp/arrays.cpp:41-44
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}

A similar test we might want to add (whenever we get around to references):
```
template 
constexpr T& getElementOf(T (&array)[N], int I) {
  return array[I];
}
static_assert(getElementOf(foo[2], 3) == &m, "");
```


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D132727#3755571 , @aaron.ballman 
wrote:

> Precommit CI found a relevant failure:

That needs the lvalue-to-rvalue conversion patch first.


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 456376.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
+
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc99-extensions"
+#pragma clang diagnostic ignored "-Winitializer-overrides"
+constexpr int DI[] = {
+  [0] = 10,
+  [1] = 20,
+  30,
+  40,
+  [1] = 50
+};
+static_assert(DI[0] == 10, "");
+static_assert(DI[1] == 50, "");
+static_assert(DI[2] == 30, "");
+static_assert(DI[3] == 40, "");
+#pragma clang diagnostic pop
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Inter

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found a relevant failure:

  FAIL: Clang :: AST/Interp/arrays.cpp (67 of 15858)
   TEST 'Clang :: AST/Interp/arrays.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -fexperimental-new-constant-interpreter -verify 
C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp
  : 'RUN: at line 2';   
c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -verify=ref 
C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include" 
"-nostdsysteminc" "-fexperimental-new-constant-interpreter" "-verify" 
"C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp"
  # command stderr:
  error: 'note' diagnostics expected but not seen: 
File C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp 
Line 49: 5 == 4
  1 error generated.
  
  error: command failed with exit status: 1
  
  --
  
  




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:43
 protected:
-  // Emitters for opcodes of various arities.
-  using NullaryFn = bool (ByteCodeExprGen::*)(const SourceInfo &);
-  using UnaryFn = bool (ByteCodeExprGen::*)(PrimType, const SourceInfo &);
-  using BinaryFn = bool (ByteCodeExprGen::*)(PrimType, PrimType,
- const SourceInfo &);
-
-  // Aliases for types defined in the emitter.
-  using LabelTy = typename Emitter::LabelTy;
-  using AddrTy = typename Emitter::AddrTy;
-
-  // Reference to a function generating the pointer of an initialized object.s
+  // Reference to a function generating the pointer of an initialized object.
   using InitFnRef = std::function;

tbaeder wrote:
> Yes, a few of these cleanups, renames and added doc comments aren't necessary 
> for this patch. I will remove them if you want to, I just don't want to 
> rebase against `main` all the time to push NFC patches.
Please remove them; we want to keep changes logically separate per our 
developer policy. The biggest reason is: we sometimes need to revert changes 
and there's no reason to lose the good (unrelated) work at the same time, but 
also, it helps code reviewers to focus on the important parts without getting 
lost tracking which changes are for what reason.



Comment at: clang/test/AST/Interp/arrays.cpp:10
+};
+
+static_assert(foo[0][0] == nullptr, "");

I'd like to see a test for designated initialization of arrays, including 
reinitialization: https://godbolt.org/z/xfjW8xPfE


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:535
+
+  // TODO: Fillers?
+  if (const auto *InitList = dyn_cast(Initializer)) {

erichkeane wrote:
> Heh, THIS is a huge "TODO" here.  The ArrayFillers go through a ton of 
> twists/turns in the current interpreter, as array-filler initializers can be 
> massive.  Do we have a way to avoid allocating space for 
> filled-but-not-referenced values in this interpreter?
Heh, believe me, I  know ;) I'd have to look into that, but I'm pretty sure the 
code as it is right now doesn't handle them at all.


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Nothing suspicious as far as I can tell, other than just punting on 
ArrayFillers.  Don't understand enough of this code to just do a straight-up 
approval though, so hopefully one of the other reviewers can take a look and 
confirm with another uninformed opinion :)




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:535
+
+  // TODO: Fillers?
+  if (const auto *InitList = dyn_cast(Initializer)) {

Heh, THIS is a huge "TODO" here.  The ArrayFillers go through a ton of 
twists/turns in the current interpreter, as array-filler initializers can be 
massive.  Do we have a way to avoid allocating space for 
filled-but-not-referenced values in this interpreter?


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:56
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+

tbaeder wrote:
> This is quite unfortunate, but it only fails when evaluating `data[0]` by 
> itself, when printing the extra info for the failed `static_assert`. I 
> roughly know what the problem is though.
Ignore this, I fixed it.


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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 455873.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, bu

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 455849.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+// TODO: There is currently a problem with how we handle
+// Pointer::toAPValue() for arrays. The following static_assert
+// will run into an assertion because we don't set the
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, but pops the pointer as well.
 template ::T>
 bool InitElemPop(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:43
 protected:
-  // Emitters for opcodes of various arities.
-  using NullaryFn = bool (ByteCodeExprGen::*)(const SourceInfo &);
-  using UnaryFn = bool (ByteCodeExprGen::*)(PrimType, const SourceInfo &);
-  using BinaryFn = bool (ByteCodeExprGen::*)(PrimType, PrimType,
- const SourceInfo &);
-
-  // Aliases for types defined in the emitter.
-  using LabelTy = typename Emitter::LabelTy;
-  using AddrTy = typename Emitter::AddrTy;
-
-  // Reference to a function generating the pointer of an initialized object.s
+  // Reference to a function generating the pointer of an initialized object.
   using InitFnRef = std::function;

Yes, a few of these cleanups, renames and added doc comments aren't necessary 
for this patch. I will remove them if you want to, I just don't want to rebase 
against `main` all the time to push NFC patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:553
+if (!visitArrayInitializer(Init))
+  return false;
+if (!this->emitPopPtr(Init))

I've been wondering about a macro to simplify code like this, e.g. `define 
DOIT(x) if(!x) return false;` that I could instead define to `assert(x)` during 
development.



Comment at: clang/test/AST/Interp/arrays.cpp:56
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+

This is quite unfortunate, but it only fails when evaluating `data[0]` by 
itself, when printing the extra info for the failed `static_assert`. I roughly 
know what the problem is though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132727

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

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It took me a pretty long time to get the patch this clean (and working), but 
here it is.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+// TODO: There is currently a problem with how we handle
+// Pointer::toAPValue() for arrays. The following static_assert
+// will run into an assertion because we don't set the
+// LValuePath correctly.
+//static_assert(data[0] == 4);
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -721,6 +721,9 @@
   return true;
 }
 
+/// 1) Pops the value from the stack
+/// 2) Peeks a pointer and gets its index \Idx
+/// 3) Sets the value on the pointer, leaving the pointer on the stack.
 template ::T>
 bool InitElem(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
@@ -732,6 +735,7 @@
   return true;
 }
 
+/// The same as InitElem, but pops the pointer as well.
 template ::T>
 bool InitElemPop(InterpState &S, CodePtr OpPC, uint32_t Idx) {
   const T &Value = S.Stk.pop();
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
==