[mlir][docgen] Display full attribute descriptions in expandable regions

[MacDue]

This updates the table of op attributes so that clicking the summary expands to show the complete description.

   Attribute | MLIR Type | Description
   <name>      <type>      ▶ <summary>  <-- Click to expand

Enum attributes have now also been updated to generate a description that lists all the cases (with both their MLIR and C++ names). This makes viewing enums on the MLIR docs much nicer.

Example

Default view:

Expanded:

---

This requires: llvm/mlir-www#158 (adds a very simple markdown shortcode)

[llvmbot]

@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-mlir-ods

@llvm/pr-subscribers-mlir

Changes

This updates the table of op attributes so that clicking the summary expands to show the complete description.

   Attribute | MLIR Type | Description
   <name>      <type>      ▶ <summary>  <-- Click to expand

Enum attributes have now also been updated to generate a description that lists all the cases (with both their MLIR and C++ names). This makes viewing enums on the MLIR docs much nicer.

Example

Default view:

Expanded:

---

This requires: llvm/mlir-www#158 (adds a very simple markdown shortcode)

---

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

2 Files Affected:

• (modified) mlir/include/mlir/IR/EnumAttr.td (+8)
• (modified) mlir/tools/mlir-tblgen/OpDocGen.cpp (+32-4)

diff --git a/mlir/include/mlir/IR/EnumAttr.td b/mlir/include/mlir/IR/EnumAttr.td
index 485c5266f3cfdfa..cb918b5eceb1a19 100644
--- a/mlir/include/mlir/IR/EnumAttr.td
+++ b/mlir/include/mlir/IR/EnumAttr.td
@@ -113,6 +113,13 @@ class I64BitEnumAttrCaseGroup<string sym, list<BitEnumAttrCaseBase> cases,
 class EnumAttrInfo<
     string name, list<EnumAttrCaseInfo> cases, Attr baseClass> :
       Attr<baseClass.predicate, baseClass.summary> {
+
+  // Generate a description of this enums members for the MLIR docs.
+  let description =
+        "Enum cases:\n" # !interleave(
+          !foreach(case, cases,
+              "* " # case.str  # " (`" # case.symbol # "`)"), "\n");
+
   // The C++ enum class name
   string className = name;
 
@@ -381,6 +388,7 @@ class EnumAttr<Dialect dialect, EnumAttrInfo enumInfo, string name = "",
                list <Trait> traits = []>
     : AttrDef<dialect, enumInfo.className, traits> {
   let summary = enumInfo.summary;
+  let description = enumInfo.description;
 
   // The backing enumeration.
   EnumAttrInfo enum = enumInfo;
diff --git a/mlir/tools/mlir-tblgen/OpDocGen.cpp b/mlir/tools/mlir-tblgen/OpDocGen.cpp
index c546763880a853f..d2538fc49b20d0d 100644
--- a/mlir/tools/mlir-tblgen/OpDocGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDocGen.cpp
@@ -173,6 +173,13 @@ static void emitOpTraitsDoc(const Operator &op, raw_ostream &os) {
   }
 }
 
+static StringRef resolveAttrDescription(const Attribute &attr) {
+  StringRef description = attr.getDescription();
+  if (description.empty())
+    return attr.getBaseAttr().getDescription();
+  return description;
+}
+
 static void emitOpDoc(const Operator &op, raw_ostream &os) {
   std::string classNameStr = op.getQualCppClassName();
   StringRef className = classNameStr;
@@ -195,13 +202,34 @@ static void emitOpDoc(const Operator &op, raw_ostream &os) {
     // TODO: Attributes are only documented by TableGen name, with no further
     // info. This should be improved.
     os << "\n#### Attributes:\n\n";
-    os << "| Attribute | MLIR Type | Description |\n"
-       << "| :-------: | :-------: | ----------- |\n";
+    // Note: This table is HTML rather than markdown so the attribute's
+    // description can appear in an expandable region. The description may be
+    // multiple lines, which is not supported in a markdown table cell.
+    os << "<table>\n";
+    // Header.
+    os << "<tr><th>Attribute</th><th>MLIR Type</th><th>Description</th></tr>\n";
     for (const auto &it : op.getAttributes()) {
       StringRef storageType = it.attr.getStorageType();
-      os << "| `" << it.name << "` | " << storageType << " | "
-         << it.attr.getSummary() << "\n";
+      // Name and storage type.
+      os << "<tr>";
+      os << "<td><code>" << it.name << "</code></td><td>" << storageType
+         << "</td><td>";
+      StringRef description = resolveAttrDescription(it.attr);
+      if (!description.empty()) {
+        // Expandable description.
+        // This appears as just the summary, but when clicked shows the full
+        // description.
+        os << "<details>"
+           << "<summary>" << it.attr.getSummary() << "</summary>"
+           << "{{% markdown %}}" << description << "{{% /markdown %}}"
+           << "</details>";
+      } else {
+        // Fallback: Single-line summary.
+        os << it.attr.getSummary();
+      }
+      os << "</td></tr>\n";
     }
+    os << "<table>\n";
   }
 
   // Emit each of the operands.

[c-rhodes]

Thanks for this Ben I think this is a nice improvement! It LGTM, but I'd like to wait for feedback from others before accepting, particularly w.r.t to emitting HTML in the doc generation.

[banach-space]

LGTM, very non-intrusive and obviously a massive improvement, thanks!

[joker-eph]

LG with the small comment from @banach-space addressed

[ScottTodd]

I'll echo the comment on the merged commit (d339d8f#commitcomment-128747641):

Hi folks. This breaks documentation generators for out-of-tree projects that don't have the custom markdown shortcode. Is there any way we could make this opt-in instead of breaking?

I can add the custom shortcode on our project downstream easily enough, but I'm just hoping upstream folks can consider us out-of-tree folks when making changes that don't really need to be backwards incompatible.

This breaks downstream projects that use other markdown renderers. Please revert and/or make this opt-in via a flag like

llvm-project/mlir/tools/mlir-tblgen/OpDocGen.cpp

Lines 36 to 44 in 9ce8103

	//===----------------------------------------------------------------------===//
	// Commandline Options
	//===----------------------------------------------------------------------===//
	static llvm::cl::OptionCategory
	docCat("Options for -gen-(attrdef|typedef|op|dialect)-doc");
	llvm::cl::opt<std::string>
	stripPrefix("strip-prefix",
	llvm::cl::desc("Strip prefix of the fully qualified names"),
	llvm::cl::init("::mlir::"), llvm::cl::cat(docCat));

Here is how one renderer handles this custom syntax (i.e. unsupported, broken):

[MacDue]

I'll add an option shortly

[ScottTodd]

Thanks, much appreciated!

[ScottTodd]

This is not valid HTML/markdown. Tags must be closed (</table>), not opened twice.