[Mlir-commits] [mlir] WIP: [mlir:python] Fix crash in `from_python` on multiple overloads (PR #191764)

Ingo Müller llvmlistbot at llvm.org
Mon Apr 13 02:24:38 PDT 2026


https://github.com/ingomueller-net updated https://github.com/llvm/llvm-project/pull/191764

>From efd75bdd2d452ca4edb7ec16f590eee69d4fdab9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ingo=20M=C3=BCller?= <ingomueller at google.com>
Date: Mon, 13 Apr 2026 08:45:27 +0200
Subject: [PATCH] [mlir:python] Fix crash in `from_python` in type casters.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR fixes a crash due to a failed assertion in the `from_python`
implementations of the type casters. The assertion obviously only
triggers if assertions are enabled, which isn't the case for many Python
installations, *and* if a Python capsule of the wrong type is attempted
to be used, so this this isn't triggered easily. The problem is that the
conversion from Python capsules may set the Python error indicator but
the callers of the type casters do not expect that. In fact, if there
are several operloads of a function, the first may cause the error
indicator to be set and the second runs into the assertion. The fix is
to unset the error indicator after a failed capsule conversion, which is
indicated with the return value of the function anyways.

In alternative fix would be to unset the error indicator *inside* the
`mlirPythonCapsuleTo*` functions; however, their documentations does say
that the Python error indicator is set, so I assume that some callers
may *want* to see the indicator and that the responsibility to handle it
is on them.

Signed-off-by: Ingo Müller <ingomueller at google.com>
---
 .../mlir/Bindings/Python/NanobindAdaptors.h   | 37 ++++++++++++-------
 ...nobind_adaptors_from_python_dirty_error.py | 36 ++++++++++++++++++
 .../python/lib/PythonTestModuleNanobind.cpp   | 30 +++++++++++++++
 3 files changed, 90 insertions(+), 13 deletions(-)
 create mode 100644 mlir/test/python/ir/nanobind_adaptors_from_python_dirty_error.py

diff --git a/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h b/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h
index 6669433550a00..68329e580975c 100644
--- a/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h
+++ b/mlir/include/mlir/Bindings/Python/NanobindAdaptors.h
@@ -72,6 +72,17 @@ mlirApiObjectToCapsule(nanobind::handle apiObject) {
   return api;
 }
 
+/// Clears the Python error indicator if the given condition `val` is false and
+/// returns the condition. This is needed in `from_python` of the type casters
+/// below, where a failed conversion from a Python capsule sets the Python error
+/// indicator but the caller of the caster expects or may even need a clean
+/// error indicator.
+inline bool pyErrClearIfFalse(bool val) {
+  if (!val)
+    PyErr_Clear();
+  return val;
+}
+
 // Note: Currently all of the following support cast from nanobind::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
@@ -85,7 +96,7 @@ struct type_caster<MlirAffineMap> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToAffineMap(capsule->ptr());
-      return !mlirAffineMapIsNull(value);
+      return pyErrClearIfFalse(!mlirAffineMapIsNull(value));
     }
     return false;
   }
@@ -108,7 +119,7 @@ struct type_caster<MlirAttribute> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToAttribute(capsule->ptr());
-      return !mlirAttributeIsNull(value);
+      return pyErrClearIfFalse(!mlirAttributeIsNull(value));
     }
     return false;
   }
@@ -131,7 +142,7 @@ struct type_caster<MlirBlock> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToBlock(capsule->ptr());
-      return !mlirBlockIsNull(value);
+      return pyErrClearIfFalse(!mlirBlockIsNull(value));
     }
     return false;
   }
@@ -159,7 +170,7 @@ struct type_caster<MlirContext> {
     }
     if (std::optional<nanobind::object> capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToContext(capsule->ptr());
-      return !mlirContextIsNull(value);
+      return pyErrClearIfFalse(!mlirContextIsNull(value));
     }
     return false;
   }
@@ -173,7 +184,7 @@ struct type_caster<MlirDialectRegistry> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToDialectRegistry(capsule->ptr());
-      return !mlirDialectRegistryIsNull(value);
+      return pyErrClearIfFalse(!mlirDialectRegistryIsNull(value));
     }
     return false;
   }
@@ -200,7 +211,7 @@ struct type_caster<MlirLocation> {
     }
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToLocation(capsule->ptr());
-      return !mlirLocationIsNull(value);
+      return pyErrClearIfFalse(!mlirLocationIsNull(value));
     }
     return false;
   }
@@ -222,7 +233,7 @@ struct type_caster<MlirModule> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToModule(capsule->ptr());
-      return !mlirModuleIsNull(value);
+      return pyErrClearIfFalse(!mlirModuleIsNull(value));
     }
     return false;
   }
@@ -246,7 +257,7 @@ struct type_caster<MlirFrozenRewritePatternSet> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToFrozenRewritePatternSet(capsule->ptr());
-      return value.ptr != nullptr;
+      return pyErrClearIfFalse(value.ptr != nullptr);
     }
     return false;
   }
@@ -269,7 +280,7 @@ struct type_caster<MlirOperation> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToOperation(capsule->ptr());
-      return !mlirOperationIsNull(value);
+      return pyErrClearIfFalse(!mlirOperationIsNull(value));
     }
     return false;
   }
@@ -293,7 +304,7 @@ struct type_caster<MlirValue> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToValue(capsule->ptr());
-      return !mlirValueIsNull(value);
+      return pyErrClearIfFalse(!mlirValueIsNull(value));
     }
     return false;
   }
@@ -319,7 +330,7 @@ struct type_caster<MlirPassManager> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToPassManager(capsule->ptr());
-      return !mlirPassManagerIsNull(value);
+      return pyErrClearIfFalse(!mlirPassManagerIsNull(value));
     }
     return false;
   }
@@ -332,7 +343,7 @@ struct type_caster<MlirTypeID> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToTypeID(capsule->ptr());
-      return !mlirTypeIDIsNull(value);
+      return pyErrClearIfFalse(!mlirTypeIDIsNull(value));
     }
     return false;
   }
@@ -356,7 +367,7 @@ struct type_caster<MlirType> {
   bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
     if (auto capsule = mlirApiObjectToCapsule(src)) {
       value = mlirPythonCapsuleToType(capsule->ptr());
-      return !mlirTypeIsNull(value);
+      return pyErrClearIfFalse(!mlirTypeIsNull(value));
     }
     return false;
   }
diff --git a/mlir/test/python/ir/nanobind_adaptors_from_python_dirty_error.py b/mlir/test/python/ir/nanobind_adaptors_from_python_dirty_error.py
new file mode 100644
index 0000000000000..8d35477aa0f34
--- /dev/null
+++ b/mlir/test/python/ir/nanobind_adaptors_from_python_dirty_error.py
@@ -0,0 +1,36 @@
+# RUN: %PYTHON %s 2>&1 | FileCheck %s
+#
+# Regression test for the dirty-error-state crash in NanobindAdaptors.h
+# from_python type casters.
+#
+# Uses an overloaded function: overload 1 takes MlirOperation, overload 2
+# takes MlirModule.  When called with an ir.Module:
+#
+#   1. nanobind tries overload 1 (MlirOperation).  from_python gets the
+#      Module's _CAPIPtr capsule, then mlirPythonCapsuleToOperation calls
+#      PyCapsule_GetPointer with "mlir.ir.Operation._CAPIPtr" — but the
+#      capsule is named "mlir.ir.Module._CAPIPtr".  PyCapsule_GetPointer
+#      returns NULL and sets PyErr_Occurred().  from_python returns false.
+#
+#   2. nanobind tries overload 2 (MlirModule).  from_python calls
+#      mlirApiObjectToCapsule → nanobind::getattr(obj, "_CAPIPtr") →
+#      _PyType_LookupRef.
+#
+# Without the fix:
+#   _PyType_LookupRef asserts !PyErr_Occurred() → SIGABRT.
+#
+# With the fix (PyErr_Clear in from_python after failed capsule conversion):
+#   Overload 2 succeeds and returns "module".
+
+from mlir import ir
+from mlir._mlir_libs import _mlirPythonTestNanobind as test_ext
+
+with ir.Context():
+    module = ir.Module.parse("module {}")
+
+    # CHECK: result = module
+    result = test_ext.take_module_or_operation(module)
+    print(f"result = {result}")
+
+# CHECK: PASS
+print("PASS")
diff --git a/mlir/test/python/lib/PythonTestModuleNanobind.cpp b/mlir/test/python/lib/PythonTestModuleNanobind.cpp
index e9754749352b1..a7a2d73738d55 100644
--- a/mlir/test/python/lib/PythonTestModuleNanobind.cpp
+++ b/mlir/test/python/lib/PythonTestModuleNanobind.cpp
@@ -147,6 +147,36 @@ NB_MODULE(_mlirPythonTestNanobind, m) {
       },
       nb::arg("context").none() = nb::none());
 
+  // Reproducer for the failed assertion `_PyType_LookupRef` triggered by
+  // `NanobindAdaptors.h::from_python` type casters.
+  //
+  // Two overloads of the same function: one takes `MlirOperation`, the other
+  // takes `MlirModule`. When called with an `ir.Module`:
+  //
+  //   1. nanobind tries overload 1 (`MlirOperation`). `from_python` calls
+  //      `mlirApiObjectToCapsule` (succeeds — `Module` has `_CAPIPtr`), then
+  //      `mlirPythonCapsuleToOperation`, whose `PyCapsule_GetPointer` fails on
+  //      the capsule-name mismatch and sets `PyErr_Occurred()`.
+  //      `from_python` returns false.
+  //
+
+  //   If `PyErr` is still set and assertions are enabled:
+  //   2. nanobind tries overload 2 (`MlirModule`).  `from_python` calls
+  //      `mlirApiObjectToCapsule` → `nanobind::getattr(obj, "_CAPIPtr")` →
+  //      CPython's `_PyType_LookupRef` → `assert(!PyErr_Occurred())` → `SIGABRT`.
+  //
+
+  //   If `PyErr_Clear` is called after failed capsule conversion:
+  //   2. `PyErr` is clear → overload 2 succeeds → returns "module".
+  m.def(
+      "take_module_or_operation",
+      [](MlirOperation) { return std::string("operation"); },
+      nb::arg("arg"));
+  m.def(
+      "take_module_or_operation",
+      [](MlirModule) { return std::string("module"); },
+      nb::arg("arg"));
+
   using namespace python_test;
   PyTestAttr::bind(m);
   PyTestType::bind(m);



More information about the Mlir-commits mailing list