[WIP] New Overload Resolution

CMakeLists.txt

@@ -10,8 +10,11 @@ include(GNUInstallDirs)
 # This option won't make a lot of sense since we only ship the shared library in site-packages
 # Perhaps this should permanently be OFF and users can build their own CppInterOp if they want to run the tests?
 option(CPPJIT_ENABLE_CPPINTEROP_TESTS "enable CppInterOp tests" OFF)
-set(CPPINTEROP_GIT_REPOSITORY "https://github.com/compiler-research/CppInterOp.git" CACHE STRING "")
-set(CPPINTEROP_GIT_TAG "f4adaa6b50542dd564089fe584d407bf15351c97" CACHE STRING "")
+# Temporary pin to the overload-resolution development branch so CI builds
+# and tests it (compiler-research/CppInterOp#1147). Returns to a pinned
+# upstream commit once that PR lands.
+set(CPPINTEROP_GIT_REPOSITORY "https://github.com/Vipul-Cariappa/CppInterOp.git" CACHE STRING "")
+set(CPPINTEROP_GIT_TAG "76c198c949d0692590bb0808c8ae10dabaeadf91" CACHE STRING "")
 set(CPPINTEROP_SOURCE_DIR "" CACHE PATH
     "Override default CppInterOp built by ExternalProject_Add, with a path to local CppInterOp source")
 
@@ -29,7 +32,7 @@ endif()
 
 # The LLVM range the pinned CppInterOp supports; update together with the tag.
 set(CPPJIT_LLVM_VERSION_MIN 20)
-set(CPPJIT_LLVM_VERSION_MAX 22)
+set(CPPJIT_LLVM_VERSION_MAX 23)
 
 # Runtime fallback anchor for the dispatch (see cppinterop_paths()): the
 # installed package under scikit-build-core, the build tree otherwise --

python/cppjit/_cpython_cppjit.py

@@ -15,6 +15,8 @@
     "_backend",
     "_begin_capture_stderr",
     "_end_capture_stderr",
+    "OverloadResolutionException",
+    "OverloadAmbiguityException",
 ]
 
 
@@ -192,6 +194,8 @@ def _add_default_paths():
 bind_object = _backend.bind_object
 nullptr = _backend.nullptr
 default = _backend.default
+OverloadResolutionException = _backend.OverloadResolutionException
+OverloadAmbiguityException = _backend.OverloadAmbiguityException
 
 
 def load_reflection_info(name):

src/cpyrt/CPPDataMember.cxx

@@ -359,8 +359,7 @@ void cpyrt::CPPDataMember::Set(interop::TCppScope_t scope,
     type = interop::GetDatamemberType(fScope);
     fFullType = interop::GetTypeAsString(type);
 
-    // Get the integer type if it's an enum
-    if (interop::IsEnumType(type))
+    if (interop::IsEnumType(type) && fFullType == "std::byte")
       type = interop::ResolveType(type);
 
     if (interop::IsConstVar(fScope))
@@ -382,17 +381,6 @@ void cpyrt::CPPDataMember::Set(interop::TCppScope_t scope,
     fDescription = PyUnicode_FromString(name.c_str());
 }
 
-//-----------------------------------------------------------------------------
-void cpyrt::CPPDataMember::Set(interop::TCppScope_t scope,
-                               const std::string& name, void* address) {
-  fEnclosingScope = scope;
-  fDescription = PyUnicode_FromString(name.c_str());
-  fOffset = (intptr_t)address;
-  fFlags = kIsStaticData | kIsConstData;
-  fConverter = CreateConverter("internal_enum_type_t");
-  fFullType = "unsigned int";
-}
-
 //-----------------------------------------------------------------------------
 void* cpyrt::CPPDataMember::GetAddress(CPPInstance* pyobj) {
   // class attributes, global properties

src/cpyrt/CPPDataMember.h

@@ -14,7 +14,6 @@ class CPPInstance;
 class CPPDataMember {
 public:
   void Set(interop::TCppScope_t scope, interop::TCppScope_t var);
-  void Set(interop::TCppScope_t scope, const std::string& name, void* address);
 
   std::string GetName();
   void* GetAddress(CPPInstance* pyobj /* owner */);
@@ -57,16 +56,6 @@ inline CPPDataMember* CPPDataMember_New(interop::TCppScope_t scope,
   return pyprop;
 }
 
-inline CPPDataMember* CPPDataMember_NewConstant(interop::TCppScope_t scope,
-                                                const std::string& name,
-                                                void* address) {
-  // Create an initialize a new property descriptor, given the C++ datum.
-  CPPDataMember* pyprop = (CPPDataMember*)CPPDataMember_Type.tp_new(
-      &CPPDataMember_Type, nullptr, nullptr);
-  pyprop->Set(scope, name, address);
-  return pyprop;
-}
-
 } // namespace cppjit::cpyrt
 
 #endif // !CPYRT_CPPDATAMEMBER_H

src/cpyrt/CPPExcInstance.h

@@ -1,6 +1,9 @@
 #ifndef CPYRT_CPPEXCINSTANCE_H
 #define CPYRT_CPPEXCINSTANCE_H
 
+#include "Python.h"
+#include "cpyrt/CommonDefs.h"
+
 //////////////////////////////////////////////////////////////////////////////
 //                                                                          //
 // cppjit::cpyrt::CPPExceptionInstance                                      //

src/cpyrt/CPPInstance.cxx

@@ -42,51 +42,6 @@ extern PyObject* gNullPtrObject;
 // pointer to the smart pointer. They carry a pointer to the Python-sode smart
 // class for dereferencing to get to the actual instance pointer.
 
-//- private helpers ----------------------------------------------------------
-namespace {
-
-// Several specific use cases require extra data in a CPPInstance, but can not
-// be a new type. E.g. cross-inheritance derived types are by definition added
-// a posterio, and caching of datamembers is up to the datamember, not the
-// instance type. To not have normal use of CPPInstance take extra memory, this
-// extended data can slot in place of fObject for those use cases.
-
-struct ExtendedData {
-  ExtendedData()
-      : fObject(nullptr), fSmartClass(nullptr), fDispatchPtr(nullptr),
-        fArraySize(0) {}
-  ~ExtendedData() {
-    for (auto& pc : fDatamemberCache)
-      Py_XDECREF(pc.second);
-    fDatamemberCache.clear();
-  }
-
-  // the original object reference it replaces (Note: has to be first data
-  // member, see usage in GetObjectRaw(), e.g. for ptr-ptr passing)
-  void* fObject;
-
-  // for caching expensive-to-create data member representations
-  cpyrt::CI_DatamemberCache_t fDatamemberCache;
-
-  // for smart pointer types
-  cpyrt::CPPSmartClass* fSmartClass;
-
-  // for back-referencing from Python-derived instances
-  cpyrt::DispatchPtr* fDispatchPtr;
-
-  // for representing T* as a low-level array
-  Py_ssize_t fArraySize;
-};
-
-} // unnamed namespace
-
-#define EXT_OBJECT(pyobj) ((ExtendedData*)((pyobj)->fObject))->fObject
-#define DATA_CACHE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDatamemberCache
-#define SMART_CLS(pyobj) ((ExtendedData*)((pyobj)->fObject))->fSmartClass
-#define SMART_TYPE(pyobj) SMART_CLS(pyobj)->fCppType
-#define DISPATCHPTR(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDispatchPtr
-#define ARRAY_SIZE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fArraySize
-
 inline void cpyrt::CPPInstance::CreateExtension() {
   if (fFlags & kIsExtended)
     return;
@@ -506,7 +461,8 @@ static inline PyObject* eqneq_binop(CPPClass* klass, PyObject* self,
     const char* cppop = op == Py_EQ ? "==" : "!=";
     PyCallable* pyfunc = FindBinaryOperator(self, obj, cppop);
     if (pyfunc)
-      binop = (PyObject*)CPPOverload_New(cppop, pyfunc);
+      binop = (PyObject*)CPPOverload_New(
+          cppop, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc);
     else {
       Py_INCREF(Py_None);
       binop = Py_None;
@@ -570,7 +526,8 @@ static inline void* cast_actual(void* obj) {
     PyCallable* pyfunc =                                                       \
         Utility::FindBinaryOperator((PyObject*)self, other, #op);              \
     if (pyfunc)                                                                \
-      ometh = (PyObject*)CPPOverload_New(#label, pyfunc);                      \
+      ometh = (PyObject*)CPPOverload_New(                                      \
+          #label, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc);  \
   }                                                                            \
   meth = ometh;
 
@@ -951,7 +908,8 @@ static PyGetSetDef op_getset[] = {
     PyErr_Clear();                                                             \
     PyCallable* pyfunc = Utility::FindBinaryOperator(left, right, #op);        \
     if (pyfunc)                                                                \
-      meth = (PyObject*)CPPOverload_New(#name, pyfunc);                        \
+      meth = (PyObject*)CPPOverload_New(                                       \
+          #name, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc);   \
     else {                                                                     \
       PyErr_SetString(PyExc_NotImplementedError, "");                          \
       return nullptr;                                                          \

src/cpyrt/CPPInstance.h

@@ -17,6 +17,8 @@
 #include "CommonDefs.h"
 #include "cppjit_interop.h"
 
+#include "cpyrt/DispatchPtr.h"
+
 // Standard
 #include <utility>
 #include <vector>
@@ -146,4 +148,49 @@ void op_dealloc_nofree(CPPInstance*);
 
 } // namespace cppjit::cpyrt
 
+//- private helpers ----------------------------------------------------------
+namespace {
+
+// Several specific use cases require extra data in a CPPInstance, but can not
+// be a new type. E.g. cross-inheritance derived types are by definition added
+// a posterio, and caching of datamembers is up to the datamember, not the
+// instance type. To not have normal use of CPPInstance take extra memory, this
+// extended data can slot in place of fObject for those use cases.
+
+struct ExtendedData {
+  ExtendedData()
+      : fObject(nullptr), fSmartClass(nullptr), fDispatchPtr(nullptr),
+        fArraySize(0) {}
+  ~ExtendedData() {
+    for (auto& pc : fDatamemberCache)
+      Py_XDECREF(pc.second);
+    fDatamemberCache.clear();
+  }
+
+  // the original object reference it replaces (Note: has to be first data
+  // member, see usage in GetObjectRaw(), e.g. for ptr-ptr passing)
+  void* fObject;
+
+  // for caching expensive-to-create data member representations
+  cppjit::cpyrt::CI_DatamemberCache_t fDatamemberCache;
+
+  // for smart pointer types
+  cppjit::cpyrt::CPPSmartClass* fSmartClass;
+
+  // for back-referencing from Python-derived instances
+  cppjit::cpyrt::DispatchPtr* fDispatchPtr;
+
+  // for representing T* as a low-level array
+  Py_ssize_t fArraySize;
+};
+
+} // unnamed namespace
+
+#define EXT_OBJECT(pyobj) ((ExtendedData*)((pyobj)->fObject))->fObject
+#define DATA_CACHE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDatamemberCache
+#define SMART_CLS(pyobj) ((ExtendedData*)((pyobj)->fObject))->fSmartClass
+#define SMART_TYPE(pyobj) SMART_CLS(pyobj)->fCppType
+#define DISPATCHPTR(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDispatchPtr
+#define ARRAY_SIZE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fArraySize
+
 #endif // !CPYRT_CPPINSTANCE_H

src/cpyrt/CPPMethod.cxx

@@ -446,144 +446,6 @@ PyObject* cpyrt::CPPMethod::Reflex(interop::Reflex::RequestId_t request,
   return PyCallable::Reflex(request, format);
 }
 
-//----------------------------------------------------------------------------
-int cpyrt::CPPMethod::GetPriority() {
-  // To help with overload selection, methods are given a priority based on the
-  // affinity of Python and C++ types. Priority only matters for methods that
-  // have an equal number of arguments and types that are possible substitutes
-  // (the normal selection mechanisms would simply distinguish them otherwise).
-
-  // The following types are ordered, in favor (variants implicit):
-  //
-  //  bool >> long >> int >> short
-  //  double >> long double >> float
-  //  const char* >> char
-  //
-  // Further, all integer types are preferred over floating point b/c int to
-  // float is allowed implicitly, float to int is not.
-  //
-  // Special cases that are disliked include void* and unknown/incomplete types.
-  // Also, moves are preferred over references. std::initializer_list is not a
-  // nice conversion candidate either, but needs to be higher priority to mix
-  // well with implicit conversions.
-  // TODO: extend this to favour classes that are not bases.
-  // TODO: profile this method (it's expensive, but should be called too often)
-
-  int priority = 0;
-
-  const size_t nArgs = interop::GetMethodNumArgs(fMethod);
-  for (int iarg = 0; iarg < (int)nArgs; ++iarg) {
-    const std::string aname = interop::GetMethodArgTypeAsString(fMethod, iarg);
-    // FIXME: convert the string comparisons with comparison to the underlying
-    // type:
-    // interop::TCppType_t type = interop::GetMethodArgType(fMethod, iarg);
-
-    // Not builtin and spelled "const void *", so match the compacted name.
-    std::string compact = aname;
-    compact.erase(std::remove(compact.begin(), compact.end(), ' '),
-                  compact.end());
-
-    if (compact.find("void*") != std::string::npos) {
-      priority -= 1000; // void*/void** shouldn't be too greedy
-    } else if (interop::IsBuiltin(aname)) {
-      // complex type (note: double penalty: for complex and the template type)
-      if (strstr(aname.c_str(), "std::complex"))
-        priority -= 10; // prefer double, float, etc. over conversion
-
-      // integer types
-      if (strstr(aname.c_str(), "bool"))
-        priority += 1; // bool over int (does accept 1 and 0)
-      else if (strstr(aname.c_str(), "long long"))
-        priority += -5; // will very likely fit
-      else if (strstr(aname.c_str(), "long"))
-        priority += -10; // most affine integer type
-      // no need to compare with int; leave at zero
-      else if (strstr(aname.c_str(), "short"))
-        priority += -50; // not really relevant as a type
-
-      // floating point types (note all numbers lower than integer types)
-      else if (strstr(aname.c_str(), "float"))
-        priority += -100; // not really relevant as a type
-      else if (strstr(aname.c_str(), "long double"))
-        priority += -90; // fits double with least loss of precision
-      else if (strstr(aname.c_str(), "double"))
-        priority += -80; // most affine floating point type
-
-      // string/char types
-      else if (strstr(aname.c_str(), "char") && aname[aname.size() - 1] != '*')
-        priority += -60; // prefer (const) char* over char
-
-    } else {
-      // This is a user-defined type (class, struct, enum, etc.).
-
-      // There's a bit of hysteresis here for templates: once GetScope() is
-      // called, their IsComplete() succeeds, the other way around it does not.
-      // Since GetPriority() is likely called several times in a sort, the
-      // GetScope() _must_ come first, or different GetPriority() calls may
-      // return different results (since the 2nd time, GetScope() will have been
-      // called from the first), killing the stable_sort.
-
-      // prefer more derived classes
-      const std::string& clean_name = TypeManip::clean_type(aname, false);
-      interop::TCppScope_t scope = interop::GetScope(clean_name);
-      if (scope)
-        priority += static_cast<int>(interop::GetNumBasesLongestBranch(scope));
-
-      // GetScope does not resolve enum names, so also detect enums by type
-      if (interop::IsEnumScope(scope) ||
-          interop::IsEnumType(interop::GetMethodArgType(fMethod, iarg)))
-        priority -= 100;
-
-      // a couple of special cases as explained above
-      if (aname.find("initializer_list") != std::string::npos) {
-        priority += 150; // needed for proper implicit conversion rules
-      } else if (aname.rfind("&&", aname.size() - 2) != std::string::npos) {
-        priority += 100; // prefer moves over other ref/ptr
-      } else if (scope && !interop::IsComplete(scope)) {
-        // class is known, but no dictionary available, 2 more cases: * and &
-        if (aname[aname.size() - 1] == '&')
-          priority += -5000;
-        else
-          priority += -2000; // prefer pointer passing over reference
-      }
-    }
-  }
-
-  // prefer methods w/o optional arguments b/c ones with optional arguments are
-  // easier to select by providing the optional arguments explicitly
-  priority += ((int)interop::GetMethodReqArgs(fMethod) - (int)nArgs);
-
-  // add a small penalty to prefer non-const methods over const ones for
-  // get/setitem
-  if (interop::IsConstMethod(fMethod) &&
-      interop::GetName(interop::TCppScope_t(fMethod.data)) == "operator[]")
-    priority += -10;
-
-  // constructors are prefered
-  if (interop::IsConstructor(fMethod))
-    priority += 100;
-
-  return priority;
-}
-
-//----------------------------------------------------------------------------
-bool cpyrt::CPPMethod::IsGreedy() {
-  // Methods will all void*-like arguments should be sorted after template
-  // instanstations, so that they don't greedily take over pointers to object.
-  // GetPriority() is too heavy-handed, as it will pull in all the argument
-  // types, so use this cheaper check.
-  const size_t nArgs = interop::GetMethodReqArgs(fMethod);
-  if (!nArgs)
-    return false;
-
-  for (int iarg = 0; iarg < (int)nArgs; ++iarg) {
-    const std::string aname = interop::GetMethodArgTypeAsString(fMethod, iarg);
-    if (aname.find("void *") != 0)
-      return false;
-  }
-  return true;
-}
-
 //----------------------------------------------------------------------------
 int cpyrt::CPPMethod::GetMaxArgs() {
   return (int)interop::GetMethodNumArgs(fMethod);