[clang][Interp] Handle PtrMemOps

[tbaeder]

Summary

This is a result of the discussions in https://reviews.llvm.org/D141472 and https://reviews.llvm.org/D142630.

We need to consider virtual functions when calling function pointers as well.

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/Context.cpp

122

Do you have test coverage for the bound member case?

[tbaeder]

clang/lib/AST/Interp/Context.cpp

122

I was sure I had, I added this to make some test work... but now I removed it and nothing breaks, so I guess I don't.

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[tbaeder]

Ping

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

213–214

Do we need similar changes for unary operators, pointer arithmetic operators, etc?

clang/lib/AST/Interp/Context.cpp

121

isFunctionType() in general?

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

234

Should we be checking !T earlier (it's used within the Discard lambda with an unprotected dereference)?

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

213–214

In what case would we need similar changes for those?

clang/lib/AST/Interp/Context.cpp

121

You mean additionally to those or as a replacement? isFunctionType() return false for function pointers it seems.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

213–214

I could use some more explanation for the changes here then; I thought this was to support overloaded binary operators. e.g.,

struct S {
  virtual constexpr int operator+(S s) const {
    return 12;
  }
};

struct T : S {
  constexpr int operator+(S s) const override {
    return 100;
  }
};

constexpr T t1, t2;
constexpr const S &s1 = t1, &s2 = t2;
constexpr const S s3, s4;
static_assert(s1 + s2 == 100);
static_assert(s3 + s4 == 12);

clang/lib/AST/Interp/Context.cpp

121

I mean instead of T->isFunctionProtoType(); a function without a prototype is still a kind of FnPtr, right? (moot for C++ constexpr, but I'm wondering why there's a requirement for a prototype or whether that's an oversight)

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

213–214

For this snippet from the tests:

constexpr S s;
constexpr decltype(&S::func) foo = &S::func;
constexpr int value = (s.*foo)();
static_assert(value == 1);

value is:

VarDecl 0x6210000a5180 <array.cpp:131:3, col:34> col:17 value 'const int' constexpr cinit
`-CXXMemberCallExpr 0x6210000a52d8 <col:25, col:34> 'int'
  `-ParenExpr 0x6210000a52b0 <col:25, col:32> '<bound member function type>'
    `-BinaryOperator 0x6210000a5288 <col:26, col:29> '<bound member function type>' '.*'
      |-DeclRefExpr 0x6210000a51f0 <col:26> 'const S':'const S' lvalue Var 0x6210000a0cf8 's' 'const S':'const S'
      `-ImplicitCastExpr 0x6210000a5268 <col:29> 'decltype(&S::func)':'int (S::*)() const' <LValueToRValue>
        `-DeclRefExpr 0x6210000a5240 <col:29> 'const decltype(&S::func)':'int (S::*const)() const' lvalue Var 0x6210000a0fc0 'foo' 'const decltype(&S::func)':'int (S::*const)() const' non_odr_use_constant

notice the BinaryOperator.

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

213–214

Ah! Now I see why we needed this, thank you!

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

234

Probably, but I changed this again a bit in https://reviews.llvm.org/D148925, so I'd like to do that change in a later commit.

[aaron.ballman]

LG!

clang/lib/AST/Interp/ByteCodeExprGen.cpp

234

At some point, we need to stop having these interdependencies -- they make reviewing the code *much* harder because the changes are split across so many patches (some of which are in progress, some already accepted but not landed, etc). The new interpreter was in a weird state regarding how little we could test with it before, but we should strive to get to fully isolated changes ASAP (I think we're pretty close).

clang/lib/AST/Interp/ByteCodeExprGen.cpp

 bool ByteCodeExprGen<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) {
   // Need short-circuiting for these.
   if (BO->isLogicalOp())
     return this->VisitLogicalBinOp(BO);
 
   const Expr *LHS = BO->getLHS();
   const Expr *RHS = BO->getRHS();
 
+  if (BO->isPtrMemOp())
+    return this->visit(RHS);
+
   // Typecheck the args.
   std::optional<PrimType> LT = classify(LHS->getType());
   std::optional<PrimType> RT = classify(RHS->getType());
   std::optional<PrimType> T = classify(BO->getType());
-  if (!LT || !RT || !T) {
-    return this->bail(BO);
-  }
 
   auto Discard = [this, T, BO](bool Result) {
     if (!Result)
       return false;
     return DiscardResult ? this->emitPop(*T, BO) : true;
   };
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
     if (!discard(LHS))
       return false;
     return Discard(this->visit(RHS));
   }
 
+  if (!LT || !RT || !T)
+    return this->bail(BO);
+
   // Pointer arithmetic special case.
   if (BO->getOpcode() == BO_Add || BO->getOpcode() == BO_Sub) {
     if (*T == PT_Ptr || (*LT == PT_Ptr && *RT == PT_Ptr))
       return this->VisitPointerArithBinOp(BO);
   }
 
   if (!visit(LHS) || !visit(RHS))
     return false;

clang/lib/AST/Interp/Context.cpp

bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
 
   Stk.clear();
   return false;
 }
 
 const LangOptions &Context::getLangOpts() const { return Ctx.getLangOpts(); }
 
 std::optional<PrimType> Context::classify(QualType T) const {
-  if (T->isFunctionPointerType() || T->isFunctionReferenceType())
-    return PT_FnPtr;
-
-  if (T->isReferenceType() || T->isPointerType())
-    return PT_Ptr;
-
   if (T->isBooleanType())
     return PT_Bool;
 
   if (T->isSignedIntegerOrEnumerationType()) {
     switch (Ctx.getIntWidth(T)) {
     case 64:
       return PT_Sint64;
     case 32:
std::optional<PrimType> Context::classify(QualType T) const {
   }
 
   if (T->isNullPtrType())
     return PT_Ptr;
 
   if (T->isFloatingType())
     return PT_Float;
 
+  if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
+      T->isFunctionProtoType())
+    return PT_FnPtr;
+
+  if (T->isReferenceType() || T->isPointerType())
+    return PT_Ptr;
+
   if (auto *AT = dyn_cast<AtomicType>(T))
     return classify(AT->getValueType());
 
+  if (auto *DT = dyn_cast<DecltypeType>(T))
+    return classify(DT->getUnderlyingType());
+
+  if (auto *DT = dyn_cast<MemberPointerType>(T))
+    return classify(DT->getPointeeType());
+
   return {};
 }
 
 unsigned Context::getCharBit() const {
   return Ctx.getTargetInfo().getCharWidth();
 }
 
 /// Simple wrapper around getFloatTypeSemantics() to make code a

clang/lib/AST/Interp/Interp.h

 
 inline bool CallPtr(InterpState &S, CodePtr OpPC) {
   const FunctionPointer &FuncPtr = S.Stk.pop<FunctionPointer>();
 
   const Function *F = FuncPtr.getFunction();
   if (!F || !F->isConstexpr())
     return false;
 
+  if (F->isVirtual())
+    return CallVirt(S, OpPC, F);
+
   return Call(S, OpPC, F);
 }
 
 inline bool GetFnPtr(InterpState &S, CodePtr &PC, const Function *Func) {
   assert(Func);
   S.Stk.push<FunctionPointer>(Func);
   return true;
 }

clang/test/AST/Interp/literals.cpp

namespace SizeOf {
 
   constexpr int F = sizeof(void); // expected-error{{incomplete type 'void'}} \
                                   // ref-error{{incomplete type 'void'}}
 
   constexpr int F2 = sizeof(gimme); // expected-error{{to a function type}} \
                                     // ref-error{{to a function type}}
 
 
-
-  /// FIXME: The following code should be accepted.
   struct S {
     void func();
   };
-  constexpr void (S::*Func)() = &S::func; // expected-error {{must be initialized by a constant expression}} \
-                                          // expected-error {{interpreter failed to evaluate an expression}}
+  constexpr void (S::*Func)() = &S::func;
   static_assert(sizeof(Func) == sizeof(&S::func), "");
 
 
   void func() {
     int n = 12;
     constexpr int oofda = sizeof(int[n++]); // expected-error {{must be initialized by a constant expression}} \
                                             // ref-error {{must be initialized by a constant expression}}
   }

clang/test/AST/Interp/records.cpp

struct SS : S {
     constexpr int foo() const override {
       return S::foo();
     }
   };
 
   constexpr SS ss;
   static_assert(ss.a == 1);
 }
+
+namespace VirtualFunctionPointers {
+  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(&S::func) foo = &S::func;
+  constexpr int value = (s.*foo)();
+  static_assert(value == 1);
+
+
+  constexpr S2 s2(1, 2, 3);
+  static_assert(s2.i == 1);
+  static_assert(s2.j == 2);
+  static_assert(s2.k == 3);
+
+  constexpr int value2 = s2.func();
+  constexpr int value3 = (s2.*foo)();
+  static_assert(value3 == 7);
+
+  constexpr int dynamicDispatch(const S &s) {
+    constexpr decltype(&S::func) SFunc = &S::func;
+
+    return (s.*SFunc)();
+  }
+
+  static_assert(dynamicDispatch(s) == 1);
+  static_assert(dynamicDispatch(s2) == 7);
+};
+
+
+
 };
 #endif