[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG976d8b40cccf: [clang][Interp] Virtual function calls 
(authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D142630?vs=527336=531697#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -380,6 +382,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}} expected-note {{declared here}}
 int Val;
   };
@@ -397,10 +400,12 @@
// ref-note {{declared here}} \
// expected-error {{must be initialized by a constant expression}} \
// expected-note {{in call to 'Derived(12)'}}
+
   static_assert(D.Val == 0, ""); // ref-error {{not an integral constant expression}} \
  // ref-note {{initializer of 'D' is not a constant expression}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -488,3 +493,201 @@
   //static_assert(b.a.m == 100, "");
   //static_assert(b.a.f == 100, "");
 }
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace VirtualDtors {
+  class A {
+  public:
+unsigned 
+constexpr A(unsigned ) : v(v) {}
+constexpr virtual ~A() {
+  v |= (1 << 0);
+}
+  };
+  class B : public A {
+  public:
+constexpr B(unsigned ) : A(v) {}
+constexpr virtual ~B() {
+  v |= (1 << 1);
+}
+  };
+  class C : public B {
+  public:
+constexpr C(unsigned ) : B(v) {}
+constexpr virtual ~C() {
+  v |= (1 << 2);
+}
+  };
+
+  constexpr bool foo() {
+unsigned a = 0;
+{
+  C c(a);
+}
+return ((a & (1 << 0)) && (a & (1 << 1)) && (a & (1 << 2)));
+  }
+
+  static_assert(foo());
+
+
+};
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; 

[PATCH] D142630: [clang][Interp] Implement virtual function calls

[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




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

tbaeder wrote:
> aaron.ballman wrote:
> > We should also have test cases for calling virtual functions from within a 
> > constructor and a destructor, as that has special semantics. e.g., 
> > https://godbolt.org/z/snaj1zfM5
> That's broken right now of course. I'l add the test and adjust the expected 
> output. I'll probably have to save a few bits for "things we're currently 
> doing" (like evaluating a constructor), but in a later patch.
> 
> FWIW, I expanded your test a bit: https://godbolt.org/z/vq5xT3xvq and it only 
> fails in clang - with a reference:
> ```
>   // CWG issue 1517: we're constructing a base class of the object described 
> by
>   // 'This', so that object has not yet begun its period of construction and
>   // any polymorphic operation on it results in undefined behavior.
> ```
Handled in a follow-up is fine by me, thanks!


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added inline comments.



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

aaron.ballman wrote:
> We should also have test cases for calling virtual functions from within a 
> constructor and a destructor, as that has special semantics. e.g., 
> https://godbolt.org/z/snaj1zfM5
That's broken right now of course. I'l add the test and adjust the expected 
output. I'll probably have to save a few bits for "things we're currently 
doing" (like evaluating a constructor), but in a later patch.

FWIW, I expanded your test a bit: https://godbolt.org/z/vq5xT3xvq and it only 
fails in clang - with a reference:
```
  // CWG issue 1517: we're constructing a base class of the object described by
  // 'This', so that object has not yet begun its period of construction and
  // any polymorphic operation on it results in undefined behavior.
```


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:1560
+  // is the furthest we might go up in the hierarchy.
+  ThisPtr = ThisPtr.getDeclPtr();
+}

tbaeder wrote:
> I think this test case was from the function pointer review, but this fixes:
> ```
> struct S {
>   virtual constexpr int func() const { return 1; }
> };
> 
> struct Middle : S {
>   constexpr Middle(int i) : i(i) {}
>   int i;
> };
> 
> struct Other {
>   constexpr Other(int k) : k(k) {}
>   int k;
> };
> 
> struct S2 : Middle, Other {
>   int j;
>   constexpr S2(int i, int j, int k) : Middle(i), Other(k), j(j) {}
>   virtual constexpr int func() const { return i + j + k  + S::func(); }
> };
> 
> 
> constexpr S s;
> constexpr decltype(::func) foo = ::func;
> constexpr S2 s2(1, 2, 3);
> constexpr int value3 = (s2.*foo)();
> ```
> 
> even I am confused by all of this now, so the comment might not be the 
> clearest :)
> 
> (I did not add that test case since it needs other changes in`classify()` and 
> `VisitBinaryOperator()`, but can do that in a follow-up commit.)
Please do add the example in a follow-up.



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

We should also have test cases for calling virtual functions from within a 
constructor and a destructor, as that has special semantics. e.g., 
https://godbolt.org/z/snaj1zfM5


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 527336.

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -380,6 +382,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}} expected-note {{declared here}}
 int Val;
   };
@@ -397,10 +400,12 @@
// ref-note {{declared here}} \
// expected-error {{must be initialized by a constant expression}} \
// expected-note {{in call to 'Derived(12)'}}
+
   static_assert(D.Val == 0, ""); // ref-error {{not an integral constant expression}} \
  // ref-note {{initializer of 'D' is not a constant expression}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -488,3 +493,158 @@
   //static_assert(b.a.m == 100, "");
   //static_assert(b.a.f == 100, "");
 }
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace VirtualDtors {
+  class A {
+  public:
+unsigned 
+constexpr A(unsigned ) : v(v) {}
+constexpr virtual ~A() {
+  v |= (1 << 0);
+}
+  };
+  class B : public A {
+  public:
+constexpr B(unsigned ) : A(v) {}
+constexpr virtual ~B() {
+  v |= (1 << 1);
+}
+  };
+  class C : public B {
+  public:
+constexpr C(unsigned ) : B(v) {}
+constexpr virtual ~C() {
+  v |= (1 << 2);
+}
+  };
+
+  constexpr bool foo() {
+unsigned a = 0;
+{
+  C c(a);
+}
+return ((a & (1 << 0)) && (a & (1 << 1)) && (a & (1 << 2)));
+  }
+
+  static_assert(foo());
+
+
+};
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}
+  };
+
+  constexpr SS ss;
+  static_assert(ss.a == 1);
+}
+};
+#endif
Index: 

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:1560
+  // is the furthest we might go up in the hierarchy.
+  ThisPtr = ThisPtr.getDeclPtr();
+}

tbaeder wrote:
> Just so I don't forget: The assignment to `ThisPtr` here is dead.
Lies. I explained above that it's for the testcase. `ThisPtr` is a reference, 
so this replaces the this pointer on the stack.


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 527331.

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -380,6 +382,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}} expected-note {{declared here}}
 int Val;
   };
@@ -397,10 +400,12 @@
// ref-note {{declared here}} \
// expected-error {{must be initialized by a constant expression}} \
// expected-note {{in call to 'Derived(12)'}}
+
   static_assert(D.Val == 0, ""); // ref-error {{not an integral constant expression}} \
  // ref-note {{initializer of 'D' is not a constant expression}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -488,3 +493,158 @@
   //static_assert(b.a.m == 100, "");
   //static_assert(b.a.f == 100, "");
 }
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace VirtualDtors {
+  class A {
+  public:
+unsigned 
+constexpr A(unsigned ) : v(v) {}
+constexpr virtual ~A() {
+  v |= (1 << 0);
+}
+  };
+  class B : public A {
+  public:
+constexpr B(unsigned ) : A(v) {}
+constexpr virtual ~B() {
+  v |= (1 << 1);
+}
+  };
+  class C : public B {
+  public:
+constexpr C(unsigned ) : B(v) {}
+constexpr virtual ~C() {
+  v |= (1 << 2);
+}
+  };
+
+  constexpr bool foo() {
+unsigned a = 0;
+{
+  C c(a);
+}
+return ((a & (1 << 0)) && (a & (1 << 1)) && (a & (1 << 2)));
+  }
+
+  static_assert(foo());
+
+
+};
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}
+  };
+
+  constexpr SS ss;
+  static_assert(ss.a == 1);
+}
+};
+#endif
Index: 

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:1560
+  // is the furthest we might go up in the hierarchy.
+  ThisPtr = ThisPtr.getDeclPtr();
+}

Just so I don't forget: The assignment to `ThisPtr` here is dead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 507648.
tbaeder added a comment.

Add some more tests for virtual destructors.


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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -308,6 +310,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}} expected-note {{declared here}}
 int Val;
   };
@@ -325,10 +328,12 @@
// ref-note {{declared here}} \
// expected-error {{must be initialized by a constant expression}} \
// expected-note {{in call to 'Derived(12)'}}
+
   static_assert(D.Val == 0, ""); // ref-error {{not an integral constant expression}} \
  // ref-note {{initializer of 'D' is not a constant expression}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -378,3 +383,158 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace VirtualDtors {
+  class A {
+  public:
+unsigned 
+constexpr A(unsigned ) : v(v) {}
+constexpr virtual ~A() {
+  v |= (1 << 0);
+}
+  };
+  class B : public A {
+  public:
+constexpr B(unsigned ) : A(v) {}
+constexpr virtual ~B() {
+  v |= (1 << 1);
+}
+  };
+  class C : public B {
+  public:
+constexpr C(unsigned ) : B(v) {}
+constexpr virtual ~C() {
+  v |= (1 << 2);
+}
+  };
+
+  constexpr bool foo() {
+unsigned a = 0;
+{
+  C c(a);
+}
+return ((a & (1 << 0)) && (a & (1 << 1)) && (a & (1 << 2)));
+  }
+
+  static_assert(foo());
+
+
+};
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

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

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -286,6 +288,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}}
 int Val;
   };
@@ -301,10 +304,12 @@
// 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}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -354,3 +359,121 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}
+  };
+
+  constexpr SS ss;
+  static_assert(ss.a == 1);
+}
+};
+#endif
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -206,6 +206,8 @@
   /// Returns the type of the innermost field.
   QualType getType() const { return getFieldDesc()->getType(); }
 
+  Pointer getDeclPtr() const { return Pointer(Pointee); }
+
   /// Returns the element size of the innermost field.
   size_t elemSize() const {
 if (Base == RootPtrMark)
Index: clang/lib/AST/Interp/Opcodes.td
===
--- 

[PATCH] D142630: [clang][Interp] Implement virtual function calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

(Still thinking about the implementation work here, mostly just drive-by 
comments at the moment. My gut instinct is that we've not got sufficient test 
coverage here, but that might be partially because of other parts of the 
interpreter not being ready yet.)




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1692-1699
+if (const auto *ME = dyn_cast(E->getCallee());
+ME && ME->hasQualifier())
+  HasQualifier = true;
+
+bool IsVirtual = false;
+if (const auto *MD = dyn_cast(FuncDecl);
+MD && MD->isVirtual())

Might as well simplify these a little bit.



Comment at: clang/lib/AST/Interp/Interp.h:1546-1547
+  ThisPtr.getDeclDesc()->getType()->getAsCXXRecordDecl();
+  const CXXRecordDecl *StaticDecl = cast(Func->getParentDecl());
+  const CXXMethodDecl *InitialFunction = cast(Func->getDecl());
+  const CXXMethodDecl *Overrider = S.getContext().getOverridingFunction(




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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:1560
+  // is the furthest we might go up in the hierarchy.
+  ThisPtr = ThisPtr.getDeclPtr();
+}

I think this test case was from the function pointer review, but this fixes:
```
struct S {
  virtual constexpr int func() const { return 1; }
};

struct Middle : S {
  constexpr Middle(int i) : i(i) {}
  int i;
};

struct Other {
  constexpr Other(int k) : k(k) {}
  int k;
};

struct S2 : Middle, Other {
  int j;
  constexpr S2(int i, int j, int k) : Middle(i), Other(k), j(j) {}
  virtual constexpr int func() const { return i + j + k  + S::func(); }
};


constexpr S s;
constexpr decltype(::func) foo = ::func;
constexpr S2 s2(1, 2, 3);
constexpr int value3 = (s2.*foo)();
```

even I am confused by all of this now, so the comment might not be the clearest 
:)

(I did not add that test case since it needs other changes in`classify()` and 
`VisitBinaryOperator()`, but can do that in a follow-up commit.)


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 497276.

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -286,6 +288,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}}
 int Val;
   };
@@ -301,10 +304,12 @@
// 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}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -354,3 +359,121 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}
+  };
+
+  constexpr SS ss;
+  static_assert(ss.a == 1);
+}
+};
+#endif
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -206,6 +206,8 @@
   /// Returns the type of the innermost field.
   QualType getType() const { return getFieldDesc()->getType(); }
 
+  Pointer getDeclPtr() const { return Pointer(Pointee); }
+
   /// Returns the element size of the innermost field.
   size_t elemSize() const {
 if (Base == RootPtrMark)
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 495789.
tbaeder set the repository for this revision to rG LLVM Github Monorepo.
tbaeder added a comment.

Fixed all the new test cases by adding a new `CallVirt` opcode and only using 
that if appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  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,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -286,6 +288,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}}
 int Val;
   };
@@ -301,10 +304,12 @@
// 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}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -354,3 +359,121 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+namespace QualifiedCalls {
+  class A {
+  public:
+  constexpr virtual int foo() const {
+  return 5;
+  }
+  };
+  class B : public A {};
+  class C : public B {
+  public:
+  constexpr int foo() const override {
+  return B::foo(); // B doesn't have a foo(), so this should call A::foo().
+  }
+  constexpr int foo2() const {
+return this->A::foo();
+  }
+  };
+  constexpr C c;
+  static_assert(c.foo() == 5);
+  static_assert(c.foo2() == 5);
+
+
+  struct S {
+int _c = 0;
+virtual constexpr int foo() const { return 1; }
+  };
+
+  struct SS : S {
+int a;
+constexpr SS() {
+  a = S::foo();
+}
+constexpr int foo() const override {
+  return S::foo();
+}
+  };
+
+  constexpr SS ss;
+  static_assert(ss.a == 1);
+}
+};
+#endif
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -182,6 +182,11 @@
   let ChangesPC = 1;
 }
 
+def CallVirt : Opcode {
+  let Args = [ArgFunction];
+  let Types = [];
+}
+
 def CallBI : Opcode {
   let Args = [ArgFunction];
   let Types = [];
Index: clang/lib/AST/Interp/InterpState.h
===
--- 

[PATCH] D142630: [clang][Interp] Implement virtual function calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D142630#4110839 , @tbaeder wrote:

> Hmm, this doesn't work if the callee is somewhere in between the path from 
> the dynamic to the static type, e.g.
>
>   struct A {
> virtual constexpr int foo() const { return 1; }
>   };
>   
>   struct B : A {
> virtual constexpr int foo() const { return A::foo(); }
>   };
>   
>   constexpr B a;
>   static_assert(a.foo() == 1);
>
> For the `A::foo()` call, the type of the `This` pointer is `B`, so we will 
> just select the `B::foo()` to call.

Ah, it sounds like we're not handling qualified calls properly. That makes me 
think of a few more test cases:

  struct A {
virtual constexpr int foo() const { return 1; }
  };
  
  struct B : A {
// Same behavior as your test case but with more baffling syntax. :-)
virtual constexpr int foo() const { return this->A::foo(); }
  };
  
  constexpr B a;
  static_assert(a.foo() == 1);

or with shadowed member variables (not related to virtual functions directly, 
but is related to qualified lookup):

  struct A {
virtual constexpr int foo() const { return 0; }
int Val = 1;
  };
  
  struct B : A {
virtual constexpr int foo() const { return A::Val; }
int Val = 2;
  };
  
  constexpr B a;
  static_assert(a.foo() == 1);

note, the rules are different for constructors and destructors than they are 
for other functions, so similar test cases involving those would be a good idea 
as well (though perhaps in a separate patch).


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Hmm, this doesn't work if the callee is somewhere in between the path from the 
dynamic to the static type, e.g.

  struct A {
virtual constexpr int foo() const { return 1; }
  };
  
  struct B : A {
virtual constexpr int foo() const { return A::foo(); }
  };
  
  constexpr B a;
  static_assert(a.foo() == 1);

For the `A::foo()` call, the type of the `This` pointer is `B`, so we will just 
select the `B::foo()` to call.


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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder updated this revision to Diff 495059.

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

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -286,6 +288,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}}
 int Val;
   };
@@ -301,10 +304,12 @@
// 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}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -354,3 +359,82 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+
+};
+#endif
Index: clang/lib/AST/Interp/InterpState.h
===
--- clang/lib/AST/Interp/InterpState.h
+++ clang/lib/AST/Interp/InterpState.h
@@ -86,6 +86,8 @@
 return M ? M->getSource(F, PC) : F->getSource(PC);
   }
 
+  Context () const { return Ctx; }
+
 private:
   /// AST Walker state.
   State 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1509,6 +1509,22 @@
 
 if (S.checkingPotentialConstantExpression())
   return false;
+
+// For a virtual call, we need to get the right function here.
+if (Func->isVirtual()) {
+  // Our ThisPtr has the decl of the right type at this point,
+  // so we just need to find the function to call.
+  const CXXRecordDecl *DynamicDecl =
+  ThisPtr.getDeclDesc()->getType()->getAsCXXRecordDecl();
+  const CXXRecordDecl *StaticDecl =
+  cast(Func->getParentDecl());
+  const CXXMethodDecl *InitialFunction =
+  cast(Func->getDecl());
+  const CXXMethodDecl *Overrider = S.getContext().getOverridingFunction(
+  DynamicDecl, StaticDecl, InitialFunction);
+  if (Overrider != InitialFunction)
+Func = S.P.getFunction(Overrider);
+}
   }
 
   if (!CheckCallable(S, PC, Func))
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -130,6 +130,13 @@
   

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

@aaron.ballman Not sure if this (the added early return) is all you meant when 
you talked about virtual destructors.

I also wasn't sure about the added `getOverridingFunction()` - I think 
`isDerivedFrom(..., paths)` is what I should be using, but that seemed 
excessively hard to use.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142630

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpState.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,8 +1,10 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple i686 -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 // RUN: %clang_cc1 -verify=ref -std=c++14 %s
+// RUN: %clang_cc1 -verify=ref -std=c++20 %s
 // RUN: %clang_cc1 -verify=ref -triple i686 %s
 
 struct BoolPair {
@@ -286,6 +288,7 @@
 };
 
 namespace DeriveFailures {
+#if __cplusplus < 202002L
   struct Base { // ref-note 2{{declared here}}
 int Val;
   };
@@ -301,10 +304,12 @@
// 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}} \
  // expected-error {{not an integral constant expression}} \
  // expected-note {{read of object outside its lifetime}}
+#endif
 
   struct AnotherBase {
 int Val;
@@ -354,3 +359,82 @@
   static_assert(getS(true).a == 12, "");
   static_assert(getS(false).a == 13, "");
 };
+
+#if __cplusplus >= 202002L
+namespace VirtualCalls {
+namespace Obvious {
+
+  class A {
+  public:
+constexpr A(){}
+constexpr virtual int foo() {
+  return 3;
+}
+  };
+  class B : public A {
+  public:
+constexpr int foo() override {
+  return 6;
+}
+  };
+
+  constexpr int getFooB(bool b) {
+A *a;
+A myA;
+B myB;
+
+if (b)
+  a = 
+else
+  a = 
+
+return a->foo();
+  }
+  static_assert(getFooB(true) == 3, "");
+  static_assert(getFooB(false) == 6, "");
+}
+
+namespace MultipleBases {
+  class A {
+  public:
+constexpr virtual int getInt() const { return 10; }
+  };
+  class B {
+  public:
+  };
+  class C : public A, public B {
+  public:
+constexpr int getInt() const override { return 20; }
+  };
+
+  constexpr int callGetInt(const A& a) { return a.getInt(); }
+  static_assert(callGetInt(C()) == 20, "");
+  static_assert(callGetInt(A()) == 10, "");
+}
+
+namespace Destructors {
+  class Base {
+  public:
+int i;
+constexpr Base(int ) : i(i) {i++;}
+constexpr virtual ~Base() {i--;}
+  };
+
+  class Derived : public Base {
+  public:
+constexpr Derived(int ) : Base(i) {}
+constexpr virtual ~Derived() {i--;}
+  };
+
+  constexpr int test() {
+int i = 0;
+Derived d(i);
+return i;
+  }
+  static_assert(test() == 1);
+}
+
+
+
+};
+#endif
Index: clang/lib/AST/Interp/InterpState.h
===
--- clang/lib/AST/Interp/InterpState.h
+++ clang/lib/AST/Interp/InterpState.h
@@ -86,6 +86,8 @@
 return M ? M->getSource(F, PC) : F->getSource(PC);
   }
 
+  Context () const { return Ctx; }
+
 private:
   /// AST Walker state.
   State 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1509,6 +1509,22 @@
 
 if (S.checkingPotentialConstantExpression())
   return false;
+
+// For a virtual call, we need to get the right function here.
+if (Func->isVirtual()) {
+  // Our ThisPtr has the decl of the right type at this point,
+  // so we just need to find the function to call.
+  const CXXRecordDecl *DynamicDecl =
+  ThisPtr.getDeclDesc()->getType()->getAsCXXRecordDecl();
+  const CXXRecordDecl *StaticDecl =
+  cast(Func->getParentDecl());
+  const CXXMethodDecl *InitialFunction =
+  cast(Func->getDecl());
+