[MLIR][Python] add/fix docstrings in IRCore

[makslevental]

This PR adds all the missing doc strings in IRCore.cpp. It also

1. Normalizes all doc strings to have proper punctuation;
2. Inlines non-duplicated docstrings which are currently at the top of the source file (and thereby possibly out of sync).

Follow-up PRs will do the same for the rest of the modules/source files.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

This PR adds all the missing doc strings in IRCore.cpp. It also

1. Normalizes all doc strings to have proper punctuation;
2. Inlines non-duplicated docstrings which are currently at the top of the source file.

Follow-up PRs will do the same for the rest of the modules/source files.

---

Patch is 96.01 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167063.diff

1 Files Affected:

• (modified) mlir/lib/Bindings/Python/IRCore.cpp (+924-590)

diff --git a/mlir/lib/Bindings/Python/IRCore.cpp b/mlir/lib/Bindings/Python/IRCore.cpp
index cda4fe19c16f8..22be38e9d8224 100644
--- a/mlir/lib/Bindings/Python/IRCore.cpp
+++ b/mlir/lib/Bindings/Python/IRCore.cpp
@@ -32,33 +32,6 @@ using llvm::SmallVector;
 using llvm::StringRef;
 using llvm::Twine;
 
-//------------------------------------------------------------------------------
-// Docstrings (trivial, non-duplicated docstrings are included inline).
-//------------------------------------------------------------------------------
-
-static const char kContextParseTypeDocstring[] =
-    R"(Parses the assembly form of a type.
-
-Returns a Type object or raises an MLIRError if the type cannot be parsed.
-
-See also: https://mlir.llvm.org/docs/LangRef/#type-system
-)";
-
-static const char kContextGetCallSiteLocationDocstring[] =
-    R"(Gets a Location representing a caller and callsite)";
-
-static const char kContextGetFileLocationDocstring[] =
-    R"(Gets a Location representing a file, line and column)";
-
-static const char kContextGetFileRangeDocstring[] =
-    R"(Gets a Location representing a file, line and column range)";
-
-static const char kContextGetFusedLocationDocstring[] =
-    R"(Gets a Location representing a fused location with optional metadata)";
-
-static const char kContextGetNameLocationDocString[] =
-    R"(Gets a Location representing a named location with optional child location)";
-
 static const char kModuleParseDocstring[] =
     R"(Parses a module's assembly format from a string.
 
@@ -67,132 +40,12 @@ Returns a new MlirModule or raises an MLIRError if the parsing fails.
 See also: https://mlir.llvm.org/docs/LangRef/
 )";
 
-static const char kModuleCAPICreate[] =
-    R"(Creates a Module from a MlirModule wrapped by a capsule (i.e. module._CAPIPtr).
-Note this returns a new object BUT _clear_mlir_module(module) must be called to
-prevent double-frees (of the underlying mlir::Module).
-)";
-
-static const char kOperationCreateDocstring[] =
-    R"(Creates a new operation.
-
-Args:
-  name: Operation name (e.g. "dialect.operation").
-  results: Sequence of Type representing op result types.
-  attributes: Dict of str:Attribute.
-  successors: List of Block for the operation's successors.
-  regions: Number of regions to create.
-  location: A Location object (defaults to resolve from context manager).
-  ip: An InsertionPoint (defaults to resolve from context manager or set to
-    False to disable insertion, even with an insertion point set in the
-    context manager).
-  infer_type: Whether to infer result types.
-Returns:
-  A new "detached" Operation object. Detached operations can be added
-  to blocks, which causes them to become "attached."
-)";
-
-static const char kOperationPrintDocstring[] =
-    R"(Prints the assembly form of the operation to a file like object.
-
-Args:
-  file: The file like object to write to. Defaults to sys.stdout.
-  binary: Whether to write bytes (True) or str (False). Defaults to False.
-  large_elements_limit: Whether to elide elements attributes above this
-    number of elements. Defaults to None (no limit).
-  large_resource_limit: Whether to elide resource attributes above this
-    number of characters. Defaults to None (no limit). If large_elements_limit
-    is set and this is None, the behavior will be to use large_elements_limit
-    as large_resource_limit.
-  enable_debug_info: Whether to print debug/location information. Defaults
-    to False.
-  pretty_debug_info: Whether to format debug information for easier reading
-    by a human (warning: the result is unparseable).
-  print_generic_op_form: Whether to print the generic assembly forms of all
-    ops. Defaults to False.
-  use_local_Scope: Whether to print in a way that is more optimized for
-    multi-threaded access but may not be consistent with how the overall
-    module prints.
-  assume_verified: By default, if not printing generic form, the verifier
-    will be run and if it fails, generic form will be printed with a comment
-    about failed verification. While a reasonable default for interactive use,
-    for systematic use, it is often better for the caller to verify explicitly
-    and report failures in a more robust fashion. Set this to True if doing this
-    in order to avoid running a redundant verification. If the IR is actually
-    invalid, behavior is undefined.
-  skip_regions: Whether to skip printing regions. Defaults to False.
-)";
-
-static const char kOperationPrintStateDocstring[] =
-    R"(Prints the assembly form of the operation to a file like object.
-
-Args:
-  file: The file like object to write to. Defaults to sys.stdout.
-  binary: Whether to write bytes (True) or str (False). Defaults to False.
-  state: AsmState capturing the operation numbering and flags.
-)";
-
-static const char kOperationGetAsmDocstring[] =
-    R"(Gets the assembly form of the operation with all options available.
-
-Args:
-  binary: Whether to return a bytes (True) or str (False) object. Defaults to
-    False.
-  ... others ...: See the print() method for common keyword arguments for
-    configuring the printout.
-Returns:
-  Either a bytes or str object, depending on the setting of the 'binary'
-  argument.
-)";
-
-static const char kOperationPrintBytecodeDocstring[] =
-    R"(Write the bytecode form of the operation to a file like object.
-
-Args:
-  file: The file like object to write to.
-  desired_version: The version of bytecode to emit.
-Returns:
-  The bytecode writer status.
-)";
-
-static const char kOperationStrDunderDocstring[] =
-    R"(Gets the assembly form of the operation with default options.
-
-If more advanced control over the assembly formatting or I/O options is needed,
-use the dedicated print or get_asm method, which supports keyword arguments to
-customize behavior.
-)";
-
 static const char kDumpDocstring[] =
-    R"(Dumps a debug representation of the object to stderr.)";
-
-static const char kAppendBlockDocstring[] =
-    R"(Appends a new block, with argument types as positional args.
-
-Returns:
-  The created block.
-)";
-
-static const char kValueDunderStrDocstring[] =
-    R"(Returns the string form of the value.
-
-If the value is a block argument, this is the assembly form of its type and the
-position in the argument list. If the value is an operation result, this is
-equivalent to printing the operation that produced it.
-)";
-
-static const char kGetNameAsOperand[] =
-    R"(Returns the string form of value as an operand (i.e., the ValueID).
-)";
-
-static const char kValueReplaceAllUsesWithDocstring[] =
-    R"(Replace all uses of value with the new value, updating anything in
-the IR that uses 'self' to use the other value instead.
-)";
+    "Dumps a debug representation of the object to stderr.";
 
 static const char kValueReplaceAllUsesExceptDocstring[] =
-    R"("Replace all uses of this value with the 'with' value, except for those
-in 'exceptions'. 'exceptions' can be either a single operation or a list of
+    R"(Replace all uses of this value with the `with` value, except for those
+in `exceptions`. 'exceptions' can be either a single operation or a list of
 operations.
 )";
 
@@ -274,22 +127,26 @@ struct PyGlobalDebugFlag {
     // Debug flags.
     nb::class_<PyGlobalDebugFlag>(m, "_GlobalDebug")
         .def_prop_rw_static("flag", &PyGlobalDebugFlag::get,
-                            &PyGlobalDebugFlag::set, "LLVM-wide debug flag")
+                            &PyGlobalDebugFlag::set, "LLVM-wide debug flag.")
         .def_static(
             "set_types",
             [](const std::string &type) {
               nb::ft_lock_guard lock(mutex);
               mlirSetGlobalDebugType(type.c_str());
             },
-            "types"_a, "Sets specific debug types to be produced by LLVM")
-        .def_static("set_types", [](const std::vector<std::string> &types) {
-          std::vector<const char *> pointers;
-          pointers.reserve(types.size());
-          for (const std::string &str : types)
-            pointers.push_back(str.c_str());
-          nb::ft_lock_guard lock(mutex);
-          mlirSetGlobalDebugTypes(pointers.data(), pointers.size());
-        });
+            "types"_a, "Sets specific debug types to be produced by LLVM.")
+        .def_static(
+            "set_types",
+            [](const std::vector<std::string> &types) {
+              std::vector<const char *> pointers;
+              pointers.reserve(types.size());
+              for (const std::string &str : types)
+                pointers.push_back(str.c_str());
+              nb::ft_lock_guard lock(mutex);
+              mlirSetGlobalDebugTypes(pointers.data(), pointers.size());
+            },
+            "types"_a,
+            "Sets multiple specific debug types to be produced by LLVM.");
   }
 
 private:
@@ -316,12 +173,18 @@ struct PyAttrBuilderMap {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyAttrBuilderMap>(m, "AttrBuilder")
-        .def_static("contains", &PyAttrBuilderMap::dunderContains)
-        .def_static("get", &PyAttrBuilderMap::dunderGetItemNamed)
+        .def_static("contains", &PyAttrBuilderMap::dunderContains,
+                    "attribute_kind"_a,
+                    "Checks whether an attribute builder is registered for the "
+                    "given attribute kind.")
+        .def_static("get", &PyAttrBuilderMap::dunderGetItemNamed,
+                    "attribute_kind"_a,
+                    "Gets the registered attribute builder for the given "
+                    "attribute kind.")
         .def_static("insert", &PyAttrBuilderMap::dunderSetItemNamed,
                     "attribute_kind"_a, "attr_builder"_a, "replace"_a = false,
                     "Register an attribute builder for building MLIR "
-                    "attributes from python values.");
+                    "attributes from Python values.");
   }
 };
 
@@ -357,8 +220,10 @@ class PyRegionIterator {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyRegionIterator>(m, "RegionIterator")
-        .def("__iter__", &PyRegionIterator::dunderIter)
-        .def("__next__", &PyRegionIterator::dunderNext);
+        .def("__iter__", &PyRegionIterator::dunderIter,
+             "Returns an iterator over the regions in the operation.")
+        .def("__next__", &PyRegionIterator::dunderNext,
+             "Returns the next region in the iteration.");
   }
 
 private:
@@ -386,7 +251,8 @@ class PyRegionList : public Sliceable<PyRegionList, PyRegion> {
   }
 
   static void bindDerived(ClassTy &c) {
-    c.def("__iter__", &PyRegionList::dunderIter);
+    c.def("__iter__", &PyRegionList::dunderIter,
+          "Returns an iterator over the regions in the sequence.");
   }
 
 private:
@@ -430,8 +296,10 @@ class PyBlockIterator {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyBlockIterator>(m, "BlockIterator")
-        .def("__iter__", &PyBlockIterator::dunderIter)
-        .def("__next__", &PyBlockIterator::dunderNext);
+        .def("__iter__", &PyBlockIterator::dunderIter,
+             "Returns an iterator over the blocks in the operation's region.")
+        .def("__next__", &PyBlockIterator::dunderNext,
+             "Returns the next block in the iteration.");
   }
 
 private:
@@ -493,10 +361,19 @@ class PyBlockList {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyBlockList>(m, "BlockList")
-        .def("__getitem__", &PyBlockList::dunderGetItem)
-        .def("__iter__", &PyBlockList::dunderIter)
-        .def("__len__", &PyBlockList::dunderLen)
-        .def("append", &PyBlockList::appendBlock, kAppendBlockDocstring,
+        .def("__getitem__", &PyBlockList::dunderGetItem,
+             "Returns the block at the specified index.")
+        .def("__iter__", &PyBlockList::dunderIter,
+             "Returns an iterator over blocks in the operation's region.")
+        .def("__len__", &PyBlockList::dunderLen,
+             "Returns the number of blocks in the operation's region.")
+        .def("append", &PyBlockList::appendBlock,
+             R"(
+              Appends a new block, with argument types as positional args.
+
+              Returns:
+                The created block.
+             )",
              nb::arg("args"), nb::kw_only(),
              nb::arg("arg_locs") = std::nullopt);
   }
@@ -527,8 +404,10 @@ class PyOperationIterator {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyOperationIterator>(m, "OperationIterator")
-        .def("__iter__", &PyOperationIterator::dunderIter)
-        .def("__next__", &PyOperationIterator::dunderNext);
+        .def("__iter__", &PyOperationIterator::dunderIter,
+             "Returns an iterator over the operations in an operation's block.")
+        .def("__next__", &PyOperationIterator::dunderNext,
+             "Returns the next operation in the iteration.");
   }
 
 private:
@@ -584,9 +463,12 @@ class PyOperationList {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyOperationList>(m, "OperationList")
-        .def("__getitem__", &PyOperationList::dunderGetItem)
-        .def("__iter__", &PyOperationList::dunderIter)
-        .def("__len__", &PyOperationList::dunderLen);
+        .def("__getitem__", &PyOperationList::dunderGetItem,
+             "Returns the operation at the specified index.")
+        .def("__iter__", &PyOperationList::dunderIter,
+             "Returns an iterator over operations in the list.")
+        .def("__len__", &PyOperationList::dunderLen,
+             "Returns the number of operations in the list.");
   }
 
 private:
@@ -609,8 +491,10 @@ class PyOpOperand {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyOpOperand>(m, "OpOperand")
-        .def_prop_ro("owner", &PyOpOperand::getOwner)
-        .def_prop_ro("operand_number", &PyOpOperand::getOperandNumber);
+        .def_prop_ro("owner", &PyOpOperand::getOwner,
+                     "Returns the operation that owns this operand.")
+        .def_prop_ro("operand_number", &PyOpOperand::getOperandNumber,
+                     "Returns the operand number in the owning operation.");
   }
 
 private:
@@ -634,8 +518,10 @@ class PyOpOperandIterator {
 
   static void bind(nb::module_ &m) {
     nb::class_<PyOpOperandIterator>(m, "OpOperandIterator")
-        .def("__iter__", &PyOpOperandIterator::dunderIter)
-        .def("__next__", &PyOpOperandIterator::dunderNext);
+        .def("__iter__", &PyOpOperandIterator::dunderIter,
+             "Returns an iterator over operands.")
+        .def("__next__", &PyOpOperandIterator::dunderNext,
+             "Returns the next operand in the iteration.");
   }
 
 private:
@@ -2979,62 +2865,103 @@ void mlir::python::populateIRCore(nb::module_ &m) {
   // Mapping of Diagnostics.
   //----------------------------------------------------------------------------
   nb::class_<PyDiagnostic>(m, "Diagnostic")
-      .def_prop_ro("severity", &PyDiagnostic::getSeverity)
-      .def_prop_ro("location", &PyDiagnostic::getLocation)
-      .def_prop_ro("message", &PyDiagnostic::getMessage)
-      .def_prop_ro("notes", &PyDiagnostic::getNotes)
-      .def("__str__", [](PyDiagnostic &self) -> nb::str {
-        if (!self.isValid())
-          return nb::str("<Invalid Diagnostic>");
-        return self.getMessage();
-      });
+      .def_prop_ro("severity", &PyDiagnostic::getSeverity,
+                   "Returns the severity of the diagnostic.")
+      .def_prop_ro("location", &PyDiagnostic::getLocation,
+                   "Returns the location associated with the diagnostic.")
+      .def_prop_ro("message", &PyDiagnostic::getMessage,
+                   "Returns the message text of the diagnostic.")
+      .def_prop_ro("notes", &PyDiagnostic::getNotes,
+                   "Returns a tuple of attached note diagnostics.")
+      .def(
+          "__str__",
+          [](PyDiagnostic &self) -> nb::str {
+            if (!self.isValid())
+              return nb::str("<Invalid Diagnostic>");
+            return self.getMessage();
+          },
+          "Returns the diagnostic message as a string.");
 
   nb::class_<PyDiagnostic::DiagnosticInfo>(m, "DiagnosticInfo")
-      .def("__init__",
-           [](PyDiagnostic::DiagnosticInfo &self, PyDiagnostic diag) {
-             new (&self) PyDiagnostic::DiagnosticInfo(diag.getInfo());
-           })
-      .def_ro("severity", &PyDiagnostic::DiagnosticInfo::severity)
-      .def_ro("location", &PyDiagnostic::DiagnosticInfo::location)
-      .def_ro("message", &PyDiagnostic::DiagnosticInfo::message)
-      .def_ro("notes", &PyDiagnostic::DiagnosticInfo::notes)
-      .def("__str__",
-           [](PyDiagnostic::DiagnosticInfo &self) { return self.message; });
+      .def(
+          "__init__",
+          [](PyDiagnostic::DiagnosticInfo &self, PyDiagnostic diag) {
+            new (&self) PyDiagnostic::DiagnosticInfo(diag.getInfo());
+          },
+          "diag"_a, "Creates a DiagnosticInfo from a Diagnostic.")
+      .def_ro("severity", &PyDiagnostic::DiagnosticInfo::severity,
+              "The severity level of the diagnostic.")
+      .def_ro("location", &PyDiagnostic::DiagnosticInfo::location,
+              "The location associated with the diagnostic.")
+      .def_ro("message", &PyDiagnostic::DiagnosticInfo::message,
+              "The message text of the diagnostic.")
+      .def_ro("notes", &PyDiagnostic::DiagnosticInfo::notes,
+              "List of attached note diagnostics.")
+      .def(
+          "__str__",
+          [](PyDiagnostic::DiagnosticInfo &self) { return self.message; },
+          "Returns the diagnostic message as a string.");
 
   nb::class_<PyDiagnosticHandler>(m, "DiagnosticHandler")
-      .def("detach", &PyDiagnosticHandler::detach)
-      .def_prop_ro("attached", &PyDiagnosticHandler::isAttached)
-      .def_prop_ro("had_error", &PyDiagnosticHandler::getHadError)
-      .def("__enter__", &PyDiagnosticHandler::contextEnter)
+      .def("detach", &PyDiagnosticHandler::detach,
+           "Detaches the diagnostic handler from the context.")
+      .def_prop_ro("attached", &PyDiagnosticHandler::isAttached,
+                   "Returns True if the handler is attached to a context.")
+      .def_prop_ro("had_error", &PyDiagnosticHandler::getHadError,
+                   "Returns True if an error was encountered during diagnostic "
+                   "handling.")
+      .def("__enter__", &PyDiagnosticHandler::contextEnter,
+           "Enters the diagnostic handler as a context manager.")
       .def("__exit__", &PyDiagnosticHandler::contextExit,
            nb::arg("exc_type").none(), nb::arg("exc_value").none(),
-           nb::arg("traceback").none());
+           nb::arg("traceback").none(),
+           "Exits the diagnostic handler context manager.");
 
   // Expose DefaultThreadPool to python
   nb::class_<PyThreadPool>(m, "ThreadPool")
-      .def("__init__", [](PyThreadPool &self) { new (&self) PyThreadPool(); })
-      .def("get_max_concurrency", &PyThreadPool::getMaxConcurrency)
-      .def("_mlir_thread_pool_ptr", &PyThreadPool::_mlir_thread_pool_ptr);
+      .def(
+          "__init__", [](PyThreadPool &self) { new (&self) PyThreadPool(); },
+          "Creates a new thread pool with default concurrency.")
+      .def("get_max_concurrency", &PyThreadPool::getMaxConcurrency,
+           "Returns the maximum number of threads in the pool.")
+      .def("_mlir_thread_pool_ptr", &PyThreadPool::_mlir_thread_pool_ptr,
+           "Returns the raw pointer to the LLVM thread pool as a string.");
 
   nb::class_<PyMlirContext>(m, "Context")
-      .def("__init__",
-           [](PyMlirContext &self) {
-             MlirContext context = mlirContextCreateWithThreading(false);
-             new (&self) PyMlirContext(context);
-           })
-      .def_static("_get_live_count", &PyMlirContext::getLiveCount)
-      .def("_get_context_again",
-           [](PyMlirContext &self) -> nb::typed<nb::object, PyMlirContext> {
-             PyMlirContextRef ref = PyMlirContext::forContext(self.get());
-             return ref.releaseObject();
-           })
-      .def("_get_live_module_count", &PyMlirContext::getLiveModuleCount)
-      .def_prop_ro(MLIR_PYTHON_CAPI_PTR_ATTR, &PyMlirContext::getCapsule)
+      .def(
+          "__init__",
+          [](PyMlirContext &self) {
+            MlirContext context = mlirContextCreateWithThreading(false);
+            new (&self) PyMlirContext(context);
+          },
+        ...
[truncated]

[makslevental]

Full disclosure: I used Claude to generate the new doc strings but I hereby do solemnly swear ✋ I checked each generated string.

[makslevental]

All of this leading whitespace is fine I think; I've checked that stubgen (which preserves comments into the type stubs) unindents and also builtin help (i.e., help(mlir.ir.Context)) does as well. @PragmaTwice can you do me a favor and double check that Sphinx/autogenapi does as well?

[PragmaTwice]

Sure. I'll try to generate a sphinx docs site for this commit.

[PragmaTwice]

Looks good.

[PragmaTwice]

Here is the generated sphinx docs for this PR: https://mlir-python-e98d3cf.surge.sh/autoapi/mlir/_mlir_libs/_mlir/ir/index.html

It seems that currently the sphinx configuration cannot handle google-style docstrings well (which is unrelated to changes of this PR.) I'll try to fix it in recent days.

[PragmaTwice]

I have solved this render issue and will open a PR to mlir-www soon. Here is the preview site (of this PR): https://mlir-python-googledocstring.surge.sh/autoapi/mlir/_mlir_libs/_mlir/ir/index.html

[jpienaar]

Is the expectation here that this is a NOP wrt what is produced? (E.g., comparing the diff of the docstrjngs generated is reasonable approach)

[jpienaar]

I would have thought in in ` quotes? (Feels akin to code construct and to be formatted mono space). But doesn't seem widely done.

[makslevental]

This one you're right on - I'll take another pass and change to ticks.

[jpienaar]

Apparently position of period here depends on US vs UK style followed. So commenting to remind myself but good.

[makslevental]

Is the expectation here that this is a NOP wrt what is produced? (E.g., comparing the diff of the docstrjngs generated is reasonable approach)

Claude fixed some grammatical errors in existing docs. Also I updated some to include proper punctuation and also updated a few that were stale.

[jpienaar]

Looks good to me.

[PragmaTwice]

Pretty big PR. Looks good at a glance. Maybe we can use AI to do a full review. 🤣

[Copilot]

Pull Request Overview

This PR enhances the Python bindings for MLIR by adding comprehensive inline docstrings to the IRCore.cpp file. The changes improve documentation quality and make the Python API more accessible to users.

Key Changes:

• Removed old docstring constants and replaced them with inline docstrings at method/property definitions
• Added detailed docstrings with parameter descriptions, return values, and usage notes for Python API methods
• Improved consistency in docstring formatting and punctuation

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[makslevental]

Ok all backticks / ' / " normalized