[mlir][Python] fix namespace shadowing on MSVC

[makslevental]

If you set MLIR_PYTHON_BINDINGS_DOMAIN=mlir, you get namespace nesting like mlir::python::mlir and then mlir::Twine shadows llvm::Twine (but only on MSVC). So prefix with ::llvm to have the correct root namespace.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

If you set MLIR_PYTHON_BINDINGS_DOMAIN=mlir, you get namespace nesting like mlir::python::mlir and then mlir::Twine shadows llvm::Twine (but only on MSVC). So prefix with ::llvm to have the correct root namespace.

---

Full diff: https://github.com/llvm/llvm-project/pull/175077.diff

1 Files Affected:

• (modified) mlir/include/mlir/Bindings/Python/IRCore.h (+11-11)

diff --git a/mlir/include/mlir/Bindings/Python/IRCore.h b/mlir/include/mlir/Bindings/Python/IRCore.h
index 59dc496c9e206..330318683c15e 100644
--- a/mlir/include/mlir/Bindings/Python/IRCore.h
+++ b/mlir/include/mlir/Bindings/Python/IRCore.h
@@ -947,7 +947,7 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteType : public BaseTy {
     if (!DerivedTy::isaFunction(orig)) {
       auto origRepr =
           nanobind::cast<std::string>(nanobind::repr(nanobind::cast(orig)));
-      throw nanobind::value_error((llvm::Twine("Cannot cast type to ") +
+      throw nanobind::value_error((::llvm::Twine("Cannot cast type to ") +
                                    DerivedTy::pyClassName + " (from " +
                                    origRepr + ")")
                                       .str()
@@ -966,7 +966,7 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteType : public BaseTy {
           if (DerivedTy::getTypeIdFunction)
             return PyTypeID(DerivedTy::getTypeIdFunction());
           throw nanobind::attribute_error(
-              (DerivedTy::pyClassName + llvm::Twine(" has no typeid."))
+              (DerivedTy::pyClassName + ::llvm::Twine(" has no typeid."))
                   .str()
                   .c_str());
         },
@@ -1080,7 +1080,7 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteAttribute : public BaseTy {
     if (!DerivedTy::isaFunction(orig)) {
       auto origRepr =
           nanobind::cast<std::string>(nanobind::repr(nanobind::cast(orig)));
-      throw nanobind::value_error((llvm::Twine("Cannot cast attribute to ") +
+      throw nanobind::value_error((::llvm::Twine("Cannot cast attribute to ") +
                                    DerivedTy::pyClassName + " (from " +
                                    origRepr + ")")
                                       .str()
@@ -1110,7 +1110,7 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteAttribute : public BaseTy {
           if (DerivedTy::getTypeIdFunction)
             return PyTypeID(DerivedTy::getTypeIdFunction());
           throw nanobind::attribute_error(
-              (DerivedTy::pyClassName + llvm::Twine(" has no typeid."))
+              (DerivedTy::pyClassName + ::llvm::Twine(" has no typeid."))
                   .str()
                   .c_str());
         },
@@ -1324,7 +1324,7 @@ class MLIR_PYTHON_API_EXPORTED PySymbolTable {
 /// Custom exception that allows access to error diagnostic information. This is
 /// converted to the `ir.MLIRError` python exception when thrown.
 struct MLIR_PYTHON_API_EXPORTED MLIRError {
-  MLIRError(llvm::Twine message,
+  MLIRError(::llvm::Twine message,
             std::vector<PyDiagnostic::DiagnosticInfo> &&errorDiagnostics = {})
       : message(message.str()), errorDiagnostics(std::move(errorDiagnostics)) {}
   std::string message;
@@ -1544,7 +1544,7 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteValue : public PyValue {
     if (!DerivedTy::isaFunction(orig.get())) {
       auto origRepr =
           nanobind::cast<std::string>(nanobind::repr(nanobind::cast(orig)));
-      throw nanobind::value_error((Twine("Cannot cast value to ") +
+      throw nanobind::value_error((::llvm::Twine("Cannot cast value to ") +
                                    DerivedTy::pyClassName + " (from " +
                                    origRepr + ")")
                                       .str()
@@ -1555,11 +1555,11 @@ class MLIR_PYTHON_API_EXPORTED PyConcreteValue : public PyValue {
 
   /// Binds the Python module objects to functions of this class.
   static void bind(nanobind::module_ &m) {
-    auto cls = ClassTy(
-        m, DerivedTy::pyClassName, nanobind::is_generic(),
-        nanobind::sig((Twine("class ") + DerivedTy::pyClassName + "(Value[_T])")
-                          .str()
-                          .c_str()));
+    auto cls = ClassTy(m, DerivedTy::pyClassName, nanobind::is_generic(),
+                       nanobind::sig((::llvm::Twine("class ") +
+                                      DerivedTy::pyClassName + "(Value[_T])")
+                                         .str()
+                                         .c_str()));
     cls.def(nanobind::init<PyValue &>(), nanobind::keep_alive<0, 1>(),
             nanobind::arg("value"));
     cls.def(

[kuhar]

thanks

[boomanaiden154]

We saw this in a downstream too. It doesn’t seem like we have upstream CI to cover this configuration? For what it’s worth, we saw this with clang-cl too.

[makslevental]

It doesn’t seem like we have upstream CI to cover this configuration

TBH I don't really understand which configuration this fails on - both IREE and substrate (@ingomueller-net) reported Windows issues but we do have Windows pre-commit right and I have basically "exhaustive" Windows (among others) tests in eudsl (where I do use clang-cl) and neither caught this. So @boomanaiden154 if you can tell me what environment/configuration produced this I'd be happy to add a test somewhere for it (probably eudsl).

[boomanaiden154]

So boomanaiden154 if you can tell me what environment/configuration produced this I'd be happy to add a test somewhere for it (probably eudsl).

I was somewhat hoping you would have the answer there despite me having no idea. 😅

I also don't have a good idea. I tried to reproduce this issue (which manifest in Jax/XLA for us) locally and could not. I sent a patch to patch in this change and someone else tested it on the CI and that worked. It was an old clang-cl version (v18), but I tried locally with that too and also could not reproduce. So not really sure what's happening.