[PATCH] D135025: [clang][Interp] Support base class constructors

[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 rG9cb4e90e7260: [clang][Interp] Support base class 
constructors (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D135025?vs=467699=469508#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -5,8 +5,6 @@
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -181,3 +179,116 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
+
+namespace MI {
+  class A {
+  public:
+int a;
+constexpr A(int a) : a(a) {}
+  };
+
+  class B {
+  public:
+int b;
+constexpr B(int b) : b(b) {}
+  };
+
+  class C : public A, public B {
+  public:
+constexpr C() : A(10), B(20) {}
+  };
+  constexpr C c = {};
+  static_assert(c.a == 10, "");
+  static_assert(c.b == 20, "");
+
+
+  class D : private A, private B {
+public:
+constexpr D() : A(20), B(30) {}
+constexpr int getA() const { return a; }
+constexpr int getB() const { return b; }
+  };
+  constexpr D d = {};
+  static_assert(d.getA() == 20, "");
+  static_assert(d.getB() == 30, "");
+};
+
+namespace DeriveFailures {
+  struct Base { // ref-note 2{{declared here}}
+int Val;
+  };
+
+  struct Derived : Base {
+int OtherVal;
+
+constexpr Derived(int i) : OtherVal(i) {} // ref-error {{never produces a constant expression}} \
+  // ref-note 2{{non-constexpr constructor 'Base' cannot be used in a constant expression}}
+  };
+
+  // FIXME: This is currently not being diagnosed with the new constant interpreter.
+  constexpr Derived D(12); // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{in call to 'Derived(12)'}} \
+   // ref-note {{declared here}} \
+   // expected-error {{must be initialized by a constant expression}}
+  static_assert(D.Val == 0, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{initializer of 'D' is not a constant expression}}
+
+#if 0
+  // FIXME: This test is currently disabled because the failing constructor call
+  //   causes us to run into an assertion later on in the new interpreter.
+  //   Once that is fixed, we fail successfully but the diagnostic uses the
+  //   wrong value.
+  struct AnotherBase {
+int Val;
+constexpr AnotherBase(int i) : Val(12 / i) {} //ref-note {{division by zero}} \
+  //expected-note {{division by zero}}
+  };
+
+  struct AnotherDerived : AnotherBase {
+constexpr AnotherDerived(int i) : AnotherBase(i) {}
+  };
+  constexpr AnotherBase Derp(0); // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{in call to 'AnotherBase(0)'}} \
+ // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{in call to 'AnotherBase(}}
+ // FIXME Previous note uses the wrong value
+#endif
+
+  struct YetAnotherBase {
+int Val;
+constexpr YetAnotherBase(int i) : Val(i) {}
+  };
+
+  struct YetAnotherDerived : YetAnotherBase {
+using YetAnotherBase::YetAnotherBase; //ref-note {{declared here}}
+int OtherVal;
+
+constexpr bool doit() const { return Val == OtherVal; }
+  };
+
+  constexpr YetAnotherDerived Oops(0); // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{constructor inherited from base class 'YetAnotherBase' cannot be used in a 

[PATCH] D135025: [clang][Interp] Support base class constructors

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

Okay, this is incremental progress, so LGTM!


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

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

In D135025#3862031 , @aaron.ballman 
wrote:

> Do you think we should address some of the FIXMEs with base class ctor 
> functionality as part of this patch, given that the failures all seem to be 
> related to that functionality? Or are these failures due to other missing 
> functionality that happened to be exposed here?

IMO it's fine to merge this. From a quick look the problems are:

1. We don't reject non-constexpr functions in a constexpr context. (this is a 
more general problem and can be tested with simple normal functions, so that's 
probably a better test)
2. In `ByteCodeExprGen::getFunction()`, we ignore any errors returned, so we 
can't print a reason for the rejection of the last test.
3. The `AnotherBase` test is rejected properly, but we don't compute the value 
that was passed to the constructor correctly. This might just be an 
uninitialized `APValue` because we abort execution, but I'd have to investigate.


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you think we should address some of the FIXMEs with base class ctor 
functionality as part of this patch, given that the failures all seem to be 
related to that functionality? Or are these failures due to other missing 
functionality that happened to be exposed here?


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[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/records.cpp:209
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};

aaron.ballman wrote:
> I'd appreciate some more failure test cases, if they're not already covered 
> elsewhere:
> ```
> struct Base {
>   int Val;
> };
> 
> struct Derived : Base {
>   int OtherVal;
> 
>   constexpr Derived(int i) : OtherVal(i) {}
> };
> 
> // Something here should be diagnosed; either because the Derived ctor is not 
> a
> // valid constexpr function or because we're accessing an uninitialized 
> member.
> constexpr Derived D(12);
> static_assert(D.Val == 0);
> 
> 
> // Another test is when a constexpr ctor calls a non-constexpr base class 
> ctor.
> struct AnotherBase {
>   int Val;
>   constexpr AnotherBase(int i) : Val(12 / i) {}
> };
> 
> struct AnotherDerived : AnotherBase {
>   constexpr AnotherDerived(int i) : AnotherBase(i) {}
> };
> constexpr AnotherDerived Derp(0);
> 
> // Skipping the derived class constructor is also
> // interesting to consider:
> struct YetAnotherBase {
>   int Val;
>   constexpr YetAnotherBase(int i) : Val(i) {}
> };
> 
> struct YetAnotherDerived : YetAnotherBase {
>   using YetAnotherBase::YetAnotherBase;
>   
>   int OtherVal;
> 
>   constexpr bool doit() const { return Val == OtherVal; }
> };
> 
> constexpr YetAnotherDerived Oops(0);
> ```
Thanks for the tests. I added them but it doesn't look very good.


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

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

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Opcodes.td
  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
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -169,3 +167,110 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
+
+namespace MI {
+  class A {
+  public:
+int a;
+constexpr A(int a) : a(a) {}
+  };
+
+  class B {
+  public:
+int b;
+constexpr B(int b) : b(b) {}
+  };
+
+  class C : public A, public B {
+  public:
+constexpr C() : A(10), B(20) {}
+  };
+  constexpr C c = {};
+  static_assert(c.a == 10);
+  static_assert(c.b == 20);
+
+
+  class D : private A, private B {
+public:
+constexpr D() : A(20), B(30) {}
+constexpr int getA() const { return a; }
+constexpr int getB() const { return b; }
+  };
+  constexpr D d = {};
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};
+
+namespace DeriveFailures {
+  struct Base { // ref-note 2{{declared here}}
+int Val;
+  };
+
+  struct Derived : Base {
+int OtherVal;
+
+constexpr Derived(int i) : OtherVal(i) {} // ref-error {{never produces a constant expression}} \
+  // ref-note 2{{non-constexpr constructor 'Base' cannot be used in a constant expression}}
+  };
+
+  // FIXME: This is currently not being diagnosed with the new constant interpreter.
+  constexpr Derived D(12); // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{in call to 'Derived(12)'}} \
+   // ref-note {{declared here}} \
+   // expected-error {{must be initialized by a constant expression}}
+  static_assert(D.Val == 0); // ref-error {{not an integral constant expression}} \
+ // ref-note {{initializer of 'D' is not a constant expression}}
+
+  struct AnotherBase {
+int Val;
+constexpr AnotherBase(int i) : Val(12 / i) {} //ref-note {{division by zero}} \
+  //expected-note {{division by zero}}
+  };
+
+  struct AnotherDerived : AnotherBase {
+constexpr AnotherDerived(int i) : AnotherBase(i) {}
+  };
+  constexpr AnotherBase Derp(0); // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{in call to 'AnotherBase(0)'}} \
+ // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{in call to 'AnotherBase(}}
+ // FIXME Previous note uses the wrong value
+
+  struct YetAnotherBase {
+int Val;
+constexpr YetAnotherBase(int i) : Val(i) {}
+  };
+
+  struct YetAnotherDerived : YetAnotherBase {
+using YetAnotherBase::YetAnotherBase; //ref-note {{declared here}}
+int OtherVal;
+
+constexpr bool doit() const { return Val == OtherVal; }
+  };
+
+  constexpr YetAnotherDerived Oops(0); // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{constructor inherited from base class 'YetAnotherBase' cannot be used in a constant expression}} \
+   // expected-error {{must be initialized by a constant expression}}
+   // FIXME: Missing reason for rejection.
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -54,6 +54,11 @@
   list Types;
 }
 
+def IntegerTypeClass : TypeClass {
+  let Types = [Sint8, Uint8, Sint16, Uint16, 

[PATCH] D135025: [clang][Interp] Support base class constructors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:258
+  /// or nullptr if no such decl exists.
+  const CXXRecordDecl * getRecordDecl(const Expr *E) const {
+QualType T = E->getType();





Comment at: clang/test/AST/Interp/records.cpp:209
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};

I'd appreciate some more failure test cases, if they're not already covered 
elsewhere:
```
struct Base {
  int Val;
};

struct Derived : Base {
  int OtherVal;

  constexpr Derived(int i) : OtherVal(i) {}
};

// Something here should be diagnosed; either because the Derived ctor is not a
// valid constexpr function or because we're accessing an uninitialized member.
constexpr Derived D(12);
static_assert(D.Val == 0);


// Another test is when a constexpr ctor calls a non-constexpr base class ctor.
struct AnotherBase {
  int Val;
  constexpr AnotherBase(int i) : Val(12 / i) {}
};

struct AnotherDerived : AnotherBase {
  constexpr AnotherDerived(int i) : AnotherBase(i) {}
};
constexpr AnotherDerived Derp(0);

// Skipping the derived class constructor is also
// interesting to consider:
struct YetAnotherBase {
  int Val;
  constexpr YetAnotherBase(int i) : Val(i) {}
};

struct YetAnotherDerived : YetAnotherBase {
  using YetAnotherBase::YetAnotherBase;
  
  int OtherVal;

  constexpr bool doit() const { return Val == OtherVal; }
};

constexpr YetAnotherDerived Oops(0);
```


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder added a comment.

Anything still missing here?


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder updated this revision to Diff 465271.

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// ref-no-diagnostics
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -157,3 +154,57 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
+
+namespace MI {
+  class A {
+  public:
+int a;
+constexpr A(int a) : a(a) {}
+  };
+
+  class B {
+  public:
+int b;
+constexpr B(int b) : b(b) {}
+  };
+
+  class C : public A, public B {
+  public:
+constexpr C() : A(10), B(20) {}
+  };
+  constexpr C c = {};
+  static_assert(c.a == 10);
+  static_assert(c.b == 20);
+
+
+  class D : private A, private B {
+public:
+constexpr D() : A(20), B(30) {}
+constexpr int getA() const { return a; }
+constexpr int getB() const { return b; }
+  };
+  constexpr D d = {};
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,31 +100,45 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
-
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->visit(InitExpr))
-  return false;
-
-if (!this->emitInitField(*T, F->Offset, InitExpr))
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
+
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->visit(InitExpr))
+return false;
+
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
+  } else if (const Type *Base = Init->getBaseClass()) {
+// Base class initializer.
+// Get This Base and call initializer on it.
+auto *BaseDecl = Base->getAsCXXRecordDecl();
+assert(BaseDecl);
+const Record::Base *B = R->getBase(BaseDecl);
+assert(B);
+if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
   return false;
-  } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
-
 if (!this->emitPopPtr(InitExpr))
   return false;
   }
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -253,6 +253,15 @@
 return (*InitFn)();
   }
 
+  /// Returns the CXXRecordDecl for the type of the given expression,
+  /// or nullptr if no such decl exists.
+  const CXXRecordDecl * 

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:165
+
+class A : public Base {
+public:

tbaeder wrote:
> erichkeane wrote:
> > tbaeder wrote:
> > > erichkeane wrote:
> > > > shafik wrote:
> > > > > How about also testing `private` and `virtual` as well as multiple 
> > > > > bases.
> > > > I like the idea of testing virtual bases as well.
> > > How would that work in a constexpr context? I get:
> > > 
> > > ```
> > > array.cpp:48:15: error: constexpr constructor not allowed in class with 
> > > virtual base class
> > > constexpr D() : A(17) {}
> > >   ^
> > > array.cpp:45:13: note: virtual base class declared here
> > >   class B : public virtual A {};
> > > ^~~~
> > > 1 error generated.
> > > ```
> > > 
> > Ah! TIL, thanks!
> Ah, this works: https://godbolt.org/z/ern3Yje9q
Checking this example, I should've tested a lot more non-constexpr stuff I 
guess.


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[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/records.cpp:165
+
+class A : public Base {
+public:

erichkeane wrote:
> tbaeder wrote:
> > erichkeane wrote:
> > > shafik wrote:
> > > > How about also testing `private` and `virtual` as well as multiple 
> > > > bases.
> > > I like the idea of testing virtual bases as well.
> > How would that work in a constexpr context? I get:
> > 
> > ```
> > array.cpp:48:15: error: constexpr constructor not allowed in class with 
> > virtual base class
> > constexpr D() : A(17) {}
> >   ^
> > array.cpp:45:13: note: virtual base class declared here
> >   class B : public virtual A {};
> > ^~~~
> > 1 error generated.
> > ```
> > 
> Ah! TIL, thanks!
Ah, this works: https://godbolt.org/z/ern3Yje9q


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:165
+
+class A : public Base {
+public:

tbaeder wrote:
> erichkeane wrote:
> > shafik wrote:
> > > How about also testing `private` and `virtual` as well as multiple bases.
> > I like the idea of testing virtual bases as well.
> How would that work in a constexpr context? I get:
> 
> ```
> array.cpp:48:15: error: constexpr constructor not allowed in class with 
> virtual base class
> constexpr D() : A(17) {}
>   ^
> array.cpp:45:13: note: virtual base class declared here
>   class B : public virtual A {};
> ^~~~
> 1 error generated.
> ```
> 
Ah! TIL, thanks!


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:263
+QualType T = E->getType();
+if (const auto *PT = dyn_cast(T))
+  T = PT->getPointeeType();

erichkeane wrote:
> We are de-pointering here, why not de-referencing? 
> 
> If we are OK with that, we can do this as:
> 
> ``` if (const auto *RD = T->getPointeeCXXRecordDecl()) 
>   return RD;
> return T->getAsCXXRecordDecl();
> ```
> and most of the work is done for you.
> 
No reason, as usual I just didn't know this was a thing. Thanks!



Comment at: clang/test/AST/Interp/records.cpp:165
+
+class A : public Base {
+public:

erichkeane wrote:
> shafik wrote:
> > How about also testing `private` and `virtual` as well as multiple bases.
> I like the idea of testing virtual bases as well.
How would that work in a constexpr context? I get:

```
array.cpp:48:15: error: constexpr constructor not allowed in class with virtual 
base class
constexpr D() : A(17) {}
  ^
array.cpp:45:13: note: virtual base class declared here
  class B : public virtual A {};
^~~~
1 error generated.
```



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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:263
+QualType T = E->getType();
+if (const auto *PT = dyn_cast(T))
+  T = PT->getPointeeType();

We are de-pointering here, why not de-referencing? 

If we are OK with that, we can do this as:

``` if (const auto *RD = T->getPointeeCXXRecordDecl()) 
  return RD;
return T->getAsCXXRecordDecl();
```
and most of the work is done for you.




Comment at: clang/test/AST/Interp/records.cpp:165
+
+class A : public Base {
+public:

shafik wrote:
> How about also testing `private` and `virtual` as well as multiple bases.
I like the idea of testing virtual bases as well.


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

This looks good to me but I would like one other set of eyes on it


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder updated this revision to Diff 464915.

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// ref-no-diagnostics
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -157,3 +154,57 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
+
+namespace MI {
+  class A {
+  public:
+int a;
+constexpr A(int a) : a(a) {}
+  };
+
+  class B {
+  public:
+int b;
+constexpr B(int b) : b(b) {}
+  };
+
+  class C : public A, public B {
+  public:
+constexpr C() : A(10), B(20) {}
+  };
+  constexpr C c = {};
+  static_assert(c.a == 10);
+  static_assert(c.b == 20);
+
+
+  class D : private A, private B {
+public:
+constexpr D() : A(20), B(30) {}
+constexpr int getA() const { return a; }
+constexpr int getB() const { return b; }
+  };
+  constexpr D d = {};
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,31 +100,45 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
-
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->visit(InitExpr))
-  return false;
-
-if (!this->emitInitField(*T, F->Offset, InitExpr))
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
+
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->visit(InitExpr))
+return false;
+
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
+  } else if (const Type *Base = Init->getBaseClass()) {
+// Base class initializer.
+// Get This Base and call initializer on it.
+auto *BaseDecl = Base->getAsCXXRecordDecl();
+assert(BaseDecl);
+const Record::Base *B = R->getBase(BaseDecl);
+assert(B);
+if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
   return false;
-  } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
-
 if (!this->emitPopPtr(InitExpr))
   return false;
   }
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -256,6 +256,16 @@
 return (*InitFn)();
   }
 
+  /// Returns the CXXRecordDecl for the type of the given expression,
+  /// or nullptr if no such decl exists.
+  CXXRecordDecl * 

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder updated this revision to Diff 464913.

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// ref-no-diagnostics
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -157,3 +154,57 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
+
+namespace MI {
+  class A {
+  public:
+int a;
+constexpr A(int a) : a(a) {}
+  };
+
+  class B {
+  public:
+int b;
+constexpr B(int b) : b(b) {}
+  };
+
+  class C : public A, public B {
+  public:
+constexpr C() : A(10), B(20) {}
+  };
+  constexpr C c = {};
+  static_assert(c.a == 10);
+  static_assert(c.b == 20);
+
+
+  class D : private A, private B {
+public:
+constexpr D() : A(20), B(30) {}
+constexpr int getA() const { return a; }
+constexpr int getB() const { return b; }
+  };
+  constexpr D d = {};
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,31 +100,45 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
-
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->visit(InitExpr))
-  return false;
-
-if (!this->emitInitField(*T, F->Offset, InitExpr))
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
+
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->visit(InitExpr))
+return false;
+
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
+  } else if (const Type *Base = Init->getBaseClass()) {
+// Base class initializer.
+// Get This Base and call initializer on it.
+auto *BaseDecl = Base->getAsCXXRecordDecl();
+assert(BaseDecl);
+const Record::Base *B = R->getBase(BaseDecl);
+assert(B);
+if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
   return false;
-  } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
-
 if (!this->emitPopPtr(InitExpr))
   return false;
   }
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -107,6 +107,21 @@
 });
   }
 
+  case CK_UncheckedDerivedToBase: {
+if (!this->visit(SubExpr))
+  return false;
+
+const CXXRecordDecl *FromDecl = getRecordDecl(SubExpr);
+assert(FromDecl);
+const 

[PATCH] D135025: [clang][Interp] Support base class constructors

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

I realize it is not directly related but I don't see it in the test suite 
(maybe I missed it) but also delegating constructors, default member 
initializaers Vs mem-init list (mem-init list has priority) and mem-init list 
items out of order of initialization.




Comment at: clang/test/AST/Interp/records.cpp:165
+
+class A : public Base {
+public:

How about also testing `private` and `virtual` as well as multiple bases.


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

https://reviews.llvm.org/D135025

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

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder updated this revision to Diff 464618.

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// ref-no-diagnostics
-// expected-no-diagnostics
-
 struct BoolPair {
   bool first;
   bool second;
@@ -157,3 +154,24 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
+static_assert(a2.i == 200, ""); // ref-error {{static assertion failed}} \
+// ref-note {{evaluates to '12 == 200'}} \
+// expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '12 == 200'}}
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,31 +100,45 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
-
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->visit(InitExpr))
-  return false;
-
-if (!this->emitInitField(*T, F->Offset, InitExpr))
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
+
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->visit(InitExpr))
+return false;
+
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
+  } else if (const Type *Base = Init->getBaseClass()) {
+// Base class initializer.
+// Get This Base and call initializer on it.
+auto *BaseDecl = Base->getAsCXXRecordDecl();
+assert(F);
+const Record::Base *B = R->getBase(BaseDecl);
+assert(B);
+if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
   return false;
-  } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
-
 if (!this->emitPopPtr(InitExpr))
   return false;
   }
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -107,6 +107,20 @@
 });
   }
 
+  case CK_UncheckedDerivedToBase: {
+if (!this->visit(SubExpr))
+  return false;
+const CXXRecordDecl *FromDecl = SubExpr->getType()->getAsCXXRecordDecl();
+assert(FromDecl);
+const CXXRecordDecl *ToDecl = CE->getType()->getAsCXXRecordDecl();
+assert(ToDecl);
+const Record *R = getRecord(FromDecl);
+const Record::Base *ToBase = R->getBase(ToDecl);
+assert(ToBase);
+
+return this->emitGetPtrBase(ToBase->Offset, CE);
+  }
+
   case CK_ArrayToPointerDecay:
   case CK_AtomicToNonAtomic:
   case CK_ConstructorConversion:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135025: [clang][Interp] Support base class constructors

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

tbaeder updated this revision to Diff 464549.

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

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -157,3 +157,20 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+  constexpr A(int a) : Base(a) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
+constexpr A a2{12};
+static_assert(a2.i == 12, "");
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,31 +100,40 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
-
-  if (Optional T = this->classify(InitExpr->getType())) {
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
+
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->visit(InitExpr))
+return false;
+
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
+  } else if (const Type *Base = Init->getBaseClass()) {
+// Base class initializer.
 if (!this->emitThis(InitExpr))
   return false;
-
-if (!this->visit(InitExpr))
-  return false;
-
-if (!this->emitInitField(*T, F->Offset, InitExpr))
-  return false;
-  } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
-if (!this->emitThis(InitExpr))
-  return false;
-
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
-
 if (!this->emitPopPtr(InitExpr))
   return false;
   }
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -115,6 +115,7 @@
   case CK_NoOp:
   case CK_UserDefinedConversion:
   case CK_NullToPointer:
+  case CK_UncheckedDerivedToBase:
 return this->Visit(SubExpr);
 
   case CK_IntegralToBoolean:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135025: [clang][Interp] Support base class constructors

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

I think this is pretty simple since we get the proper constructor as out 
`InitExpr` anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135025

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -157,3 +157,17 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,28 +100,39 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
 
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
 
-if (!this->visit(InitExpr))
-  return false;
+  if (!this->visit(InitExpr))
+return false;
 
-if (!this->emitInitField(*T, F->Offset, InitExpr))
-  return false;
+  if (!this->emitInitField(*T, F->Offset, InitExpr))
+return false;
+} else {
+  // Non-primitive case. Get a pointer to the field-to-initialize
+  // on the stack and call visitInitialzer() for it.
+  if (!this->emitThis(InitExpr))
+return false;
+
+  if (!this->emitGetPtrField(F->Offset, InitExpr))
+return false;
+
+  if (!this->visitInitializer(InitExpr))
+return false;
+
+  if (!this->emitPopPtr(InitExpr))
+return false;
+}
   } else {
-// Non-primitive case. Get a pointer to the field-to-initialize
-// on the stack and call visitInitialzer() for it.
+// Base class constructor.
 if (!this->emitThis(InitExpr))
   return false;
 
-if (!this->emitGetPtrField(F->Offset, InitExpr))
-  return false;
-
 if (!this->visitInitializer(InitExpr))
   return false;
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -115,6 +115,7 @@
   case CK_NoOp:
   case CK_UserDefinedConversion:
   case CK_NullToPointer:
+  case CK_UncheckedDerivedToBase:
 return this->Visit(SubExpr);
 
   case CK_IntegralToBoolean:


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -157,3 +157,17 @@
 static_assert(LT2.v[0].second == false, "");
 static_assert(LT2.v[2].first == true, "");
 static_assert(LT2.v[2].second == false, "");
+
+class Base {
+public:
+  int i;
+  constexpr Base() : i(10) {}
+  constexpr Base(int i) : i(i) {}
+};
+
+class A : public Base {
+public:
+  constexpr A() : Base(100) {}
+};
+constexpr A a{};
+static_assert(a.i == 100, "");
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -100,28 +100,39 @@
 const Record *R = this->getRecord(RD);
 
 for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
   const Expr *InitExpr = Init->getInit();
-  const Record::Field *F = R->getField(Member);
+  if (const FieldDecl *Member = Init->getMember()) {
+const Record::Field *F = R->getField(Member);
 
-  if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitThis(InitExpr))
-  return false;
+if (Optional T = this->classify(InitExpr->getType())) {
+  if (!this->emitThis(InitExpr))
+return false;
 
-if