Skip to content

Commit bd9651b

Browse files
authored
[mlir][py] avoid crashing on None contexts in custom gets (#171140)
Following a series of refactorings, MLIR Python bindings would crash if a dialect object requiring a context defined using mlir_attribute/type_subclass was constructed outside of the `ir.Context` context manager. The type caster for `MlirContext` would try using `ir.Context.current` when the default `None` value was provided to the `get`, which would also just return `None`. The caster would then attempt to obtain the MLIR capsule for that `None`, fail, but access it anyway without checking, leading to a C++ assertion failure or segfault. Guard against this case in nanobind adaptors. Also emit a warning to the user to clarify expectations, as the default message confusingly says that `None` is accepted as context and then fails with a type error. Using Python C API is currently recommended by nanobind in this case since the surrounding function must be marked `noexcept`. The corresponding test is in the PDL dialect since it is where I first observed the behavior. Core types are not using the `mlir_type_subclass` mechanism and are immune to the problem, so cannot be used for checking.
1 parent 857b68f commit bd9651b

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

mlir/include/mlir/Bindings/Python/NanobindAdaptors.h

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,14 +181,22 @@ struct type_caster<MlirContext> {
181181
bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
182182
if (src.is_none()) {
183183
// Gets the current thread-bound context.
184-
// TODO: This raises an error of "No current context" currently.
185-
// Update the implementation to pretty-print the helpful error that the
186-
// core implementations print in this case.
187184
src = mlir::python::irModule().attr("Context").attr("current");
188185
}
189-
std::optional<nanobind::object> capsule = mlirApiObjectToCapsule(src);
190-
value = mlirPythonCapsuleToContext(capsule->ptr());
191-
return !mlirContextIsNull(value);
186+
// If there is no context, including thread-bound, emit a warning (since
187+
// this function is not allowed to throw) and fail to cast.
188+
if (src.is_none()) {
189+
PyErr_Warn(
190+
PyExc_RuntimeWarning,
191+
"Passing None as MLIR Context is only allowed inside "
192+
"the " MAKE_MLIR_PYTHON_QUALNAME("ir.Context") " context manager.");
193+
return false;
194+
}
195+
if (std::optional<nanobind::object> capsule = mlirApiObjectToCapsule(src)) {
196+
value = mlirPythonCapsuleToContext(capsule->ptr());
197+
return !mlirContextIsNull(value);
198+
}
199+
return false;
192200
}
193201
};
194202

mlir/test/python/dialects/pdl_types.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,16 @@ def test_value_type():
148148
print(parsedType)
149149
# CHECK: !pdl.value
150150
print(constructedType)
151+
152+
153+
# CHECK-LABEL: TEST: test_type_without_context
154+
@run
155+
def test_type_without_context():
156+
# Constructing a type without the surrounding ir.Context context manager
157+
# should raise an exception but not crash.
158+
try:
159+
constructedType = pdl.ValueType.get()
160+
except TypeError:
161+
pass
162+
else:
163+
assert False, "Expected TypeError to be raised."

0 commit comments

Comments
 (0)