[mlir] Better error message in PybindAdaptors.h

[ftynse]

Summary

When attempting to cast a pybind11 handle to an MLIR C API object through
capsules, the binding code would attempt to directly access the "_CAPIPtr"
attribute on the object, leading to a rather obscure AttributeError when the
attribute was missing, e.g., on non-MLIR types. Check for its presence and
throw a TypeError instead.

Depends On D117646

[stellaraccident]

mlir/include/mlir/Bindings/Python/PybindAdaptors.h

58

I wonder if this shouldn't be a try-catch around this vs a double attribute lookup? That would make it zero cost on the success case.

mlir/include/mlir/Bindings/Python/PybindAdaptors.h

 
 template <typename T>
 struct type_caster<llvm::Optional<T>> : optional_caster<llvm::Optional<T>> {};
 
 /// Helper to convert a presumed MLIR API object to a capsule, accepting either
 /// an explicit Capsule (which can happen when two C APIs are communicating
 /// directly via Python) or indirectly by querying the MLIR_PYTHON_CAPI_PTR_ATTR
 /// attribute (through which supported MLIR Python API objects export their
-/// contained API pointer as a capsule). This is intended to be used from
-/// type casters, which are invoked with a raw handle (unowned). The returned
-/// object's lifetime may not extend beyond the apiObject handle without
-/// explicitly having its refcount increased (i.e. on return).
+/// contained API pointer as a capsule). Throws a type error if the object is
+/// neither. This is intended to be used from type casters, which are invoked
+/// with a raw handle (unowned). The returned object's lifetime may not extend
+/// beyond the apiObject handle without explicitly having its refcount increased
+/// (i.e. on return).
 static py::object mlirApiObjectToCapsule(py::handle apiObject) {
   if (PyCapsule_CheckExact(apiObject.ptr()))
     return py::reinterpret_borrow<py::object>(apiObject);
+  if (!py::hasattr(apiObject, MLIR_PYTHON_CAPI_PTR_ATTR)) {
+    auto repr = py::repr(apiObject).cast<std::string>();
+    throw py::type_error(
+        (llvm::Twine("Expected an MLIR object (got ") + repr + ").").str());
+  }
   return apiObject.attr(MLIR_PYTHON_CAPI_PTR_ATTR);
 }
 
 // Note: Currently all of the following support cast from py::object to the
 // Mlir* C-API type, but only a few light-weight, context-bound ones
 // implicitly cast the other way because the use case has not yet emerged and
 // ownership is unclear.
 
 /// Casts object <-> MlirAffineMap.

mlir/test/python/dialects/python_test.py

with Context() as ctx:
     unit = UnitAttr.get()
     try:
       test.TestAttr(unit)
     except ValueError as e:
       assert "Cannot cast attribute to TestAttr" in str(e)
     else:
       raise
 
+    # The following must trigger a TypeError from our adaptors and must not
+    # crash.
+    try:
+      test.TestAttr(42)
+    except TypeError as e:
+      assert "Expected an MLIR object" in str(e)
+    else:
+      raise
+
     # The following must trigger a TypeError from pybind (therefore, not
     # checking its message) and must not crash.
     try:
       test.TestAttr(42, 56)
     except TypeError:
       pass
     else:
       raise
with Context() as ctx:
     i8 = IntegerType.get_signless(8)
     try:
       test.TestType(i8)
     except ValueError as e:
       assert "Cannot cast type to TestType" in str(e)
     else:
       raise
 
+    # The following must trigger a TypeError from our adaptors and must not
+    # crash.
+    try:
+      test.TestType(42)
+    except TypeError as e:
+      assert "Expected an MLIR object" in str(e)
+    else:
+      raise
+
     # The following must trigger a TypeError from pybind (therefore, not
     # checking its message) and must not crash.
     try:
       test.TestType(42, 56)
     except TypeError:
       pass
     else:
       raise