[mlir][ods] Simplify signature of `custom` printers and parsers of Attributes and Types in presence of default constructible parameters

[zero9178]

Summary

The vast majority of parameters of C++ types used as parameters for Attributes and Types are likely to be default constructible. Nevertheless, TableGen conservatively generates code for the custom directive, expecting signatures using FailureOr<T> for all parameter types T to accomodate them possibly not being default constructible. This however reduces the ergonomics of the likely case of default constructible parameters.

This patch fixes that issue, while barely changing the generated TableGen code, by using a helper function that is used to pass any parameters into custom parser methods. If the type is default constructible, as deemed by the C++ compiler, a default constructible instance is created and passed into the parser method by reference. In all other cases it is a Noop and a FailureOr is passed as before.

Documentation was also updated to document the new behaviour.

Fixes https://github.com/llvm/llvm-project/issues/60178

[zero9178]

clang-format

[mehdi_amini]

Nice!

mlir/docs/DefiningDialects/AttributesAndTypes.md

 let parameters = (ins "int":$foo, "int":$bar);
 let assemblyFormat = "custom<Foo>($foo) custom<Bar>($bar, ref($foo))";
 ```
 
 The `custom` directive `custom<Foo>($foo)` will in the parser and printer
 respectively generate calls to:
 
 ```c++
-LogicalResult parseFoo(AsmParser &parser, FailureOr<int> &foo);
+LogicalResult parseFoo(AsmParser &parser, int &foo);
 void printFoo(AsmPrinter &printer, int foo);
 ```
 
+As you can see, by default parameters are passed into the parse function by 
+reference. This is only possible if the C++ type is default constructible.
+If the C++ type is not default constructible, the parameter is wrapped in a
+`FailureOr`. Therefore, given the following definition: 
+
+```tablegen
+let parameters = (ins "NotDefaultConstructible":$foobar);
+let assemblyFormat = "custom<Fizz>($foobar)";
+```
+
+It will generate calls expecting the following signature for `parseFizz`:
+
+```c++
+LogicalResult parseFizz(AsmParser &parser, FailureOr<NotDefaultConstructible> &foobar);
+```
+
 A previously bound variable can be passed as a parameter to a `custom` directive
 by wrapping it in a `ref` directive. In the previous example, `$foo` is bound by
 the first directive. The second directive references it and expects the
 following printer and parser signatures:
 
 ```c++
-LogicalResult parseBar(AsmParser &parser, FailureOr<int> &bar, int foo);
+LogicalResult parseBar(AsmParser &parser, int &bar, int foo);
 void printBar(AsmPrinter &printer, int bar, int foo);
 ```
 
 More complex C++ types can be used with the `custom` directive. The only caveat
 is that the parameter for the parser must use the storage type of the parameter.
 For example, `StringRefParameter` expects the parser and printer signatures as:
 
 ```c++
-LogicalResult parseStringParam(AsmParser &parser,
-                               FailureOr<std::string> &value);
+LogicalResult parseStringParam(AsmParser &parser, std::string &value);
 void printStringParam(AsmPrinter &printer, StringRef value);
 ```
 
 The custom parser is considered to have failed if it returns failure or if any
 bound parameters have failure values afterwards.
 
 A string of C++ code can be used as a `custom` directive argument. When
 generating the custom parser and printer call, the string is pasted as a

mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.h

 /// Parses an LLVM dialect type.
 Type parseType(DialectAsmParser &parser);
 
 /// Prints an LLVM Dialect type.
 void printType(Type type, AsmPrinter &printer);
 } // namespace detail
 
 /// Parse any MLIR type or a concise syntax for LLVM types.
-ParseResult parsePrettyLLVMType(AsmParser &p, FailureOr<Type> &type);
+ParseResult parsePrettyLLVMType(AsmParser &p, Type &type);
 /// Print any MLIR type or a concise syntax for LLVM types.
 void printPrettyLLVMType(AsmPrinter &p, Type type);
 
 //===----------------------------------------------------------------------===//
 // Utility functions.
 //===----------------------------------------------------------------------===//
 
 /// Returns `true` if the given type is compatible with the LLVM dialect. This

mlir/include/mlir/IR/AttributeSupport.h

ctx->getAttributeUniquer()
             });
   }
 
 private:
   /// Initialize the given attribute storage instance.
   static void initializeAttributeStorage(AttributeStorage *storage,
                                          MLIRContext *ctx, TypeID attrID);
 };
+
+// Internal function called by ODS generated code.
+// Default initializes the type within a FailureOr<T> if T is default
+// constructible and returns a reference to the instance.
+// Otherwise, returns a reference to the FailureOr<T>.
+template <class T>
+decltype(auto) unwrapForCustomParse(FailureOr<T> &failureOr) {
+  if constexpr (std::is_default_constructible_v<T>)
+    return failureOr.emplace();
+  else
+    return failureOr;
+}
+
 } // namespace detail
 
 } // namespace mlir
 
 #endif

mlir/lib/Dialect/LLVMIR/IR/LLVMTypeSyntax.cpp

if (!type)
     return type;
   if (!isCompatibleOuterType(type)) {
     parser.emitError(loc) << "unexpected type, expected keyword";
     return nullptr;
   }
   return type;
 }
 
-ParseResult LLVM::parsePrettyLLVMType(AsmParser &p, FailureOr<Type> &type) {
-  type.emplace();
-  return dispatchParse(p, *type);
+ParseResult LLVM::parsePrettyLLVMType(AsmParser &p, Type &type) {
+  return dispatchParse(p, type);
 }
 
 void LLVM::printPrettyLLVMType(AsmPrinter &p, Type type) {
   return dispatchPrint(p, type);
 }

mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp

 using namespace mlir::LLVM;
 
 constexpr const static unsigned kBitsInByte = 8;
 
 //===----------------------------------------------------------------------===//
 // custom<FunctionTypes>
 //===----------------------------------------------------------------------===//
 
-static ParseResult parseFunctionTypes(AsmParser &p,
-                                      FailureOr<SmallVector<Type>> &params,
-                                      FailureOr<bool> &isVarArg) {
-  params.emplace();
+static ParseResult parseFunctionTypes(AsmParser &p, SmallVector<Type> &params,
+                                      bool &isVarArg) {
   isVarArg = false;
   // `(` `)`
   if (succeeded(p.parseOptionalRParen()))
     return success();
 
   // `(` `...` `)`
   if (succeeded(p.parseOptionalEllipsis())) {
     isVarArg = true;
     return p.parseRParen();
   }
 
   // type (`,` type)* (`,` `...`)?
-  FailureOr<Type> type;
+  Type type;
   if (parsePrettyLLVMType(p, type))
     return failure();
-  params->push_back(*type);
+  params.push_back(type);
   while (succeeded(p.parseOptionalComma())) {
     if (succeeded(p.parseOptionalEllipsis())) {
       isVarArg = true;
       return p.parseRParen();
     }
     if (parsePrettyLLVMType(p, type))
       return failure();
-    params->push_back(*type);
+    params.push_back(type);
   }
   return p.parseRParen();
 }
 
 static void printFunctionTypes(AsmPrinter &p, ArrayRef<Type> params,
                                bool isVarArg) {
   llvm::interleaveComma(params, p,
                         [&](Type type) { printPrettyLLVMType(p, type); });
   if (isVarArg) {
     if (!params.empty())
       p << ", ";
     p << "...";
   }
   p << ')';
 }
 
 //===----------------------------------------------------------------------===//
 // custom<Pointer>
 //===----------------------------------------------------------------------===//
 
-static ParseResult parsePointer(AsmParser &p, FailureOr<Type> &elementType,
-                                FailureOr<unsigned> &addressSpace) {
-  addressSpace = 0;
+static ParseResult parsePointer(AsmParser &p, Type &elementType,
+                                unsigned &addressSpace) {
   // `<` addressSpace `>`
-  OptionalParseResult result = p.parseOptionalInteger(*addressSpace);
+  OptionalParseResult result = p.parseOptionalInteger(addressSpace);
   if (result.has_value()) {
     if (failed(result.value()))
       return failure();
     elementType = Type();
     return success();
   }
 
   if (parsePrettyLLVMType(p, elementType))
     return failure();
   if (succeeded(p.parseOptionalComma()))
-    return p.parseInteger(*addressSpace);
+    return p.parseInteger(addressSpace);
 
   return success();
 }
 
 static void printPointer(AsmPrinter &p, Type elementType,
                          unsigned addressSpace) {
   if (elementType)
     printPrettyLLVMType(p, elementType);

mlir/test/lib/Dialect/Test/TestAttributes.cpp

if (auto *blob = getHandle().getBlob())
     return blob->getDataAs<uint64_t>();
   return std::nullopt;
 }
 
 //===----------------------------------------------------------------------===//
 // TestCustomAnchorAttr
 //===----------------------------------------------------------------------===//
 
-static ParseResult parseTrueFalse(AsmParser &p,
-                                  FailureOr<std::optional<int>> &result) {
+static ParseResult parseTrueFalse(AsmParser &p, std::optional<int> &result) {
   bool b;
   if (p.parseInteger(b))
     return failure();
-  result = std::optional<int>(b);
+  result = b;
   return success();
 }
 
 static void printTrueFalse(AsmPrinter &p, std::optional<int> result) {
   p << (*result ? "true" : "false");
 }
 
 //===----------------------------------------------------------------------===//

mlir/test/lib/Dialect/Test/TestTypes.cpp

 static llvm::hash_code test::hash_value(const FieldInfo &fi) { // NOLINT
   return llvm::hash_combine(fi.name, fi.type);
 }
 
 //===----------------------------------------------------------------------===//
 // TestCustomType
 //===----------------------------------------------------------------------===//
 
-static LogicalResult parseCustomTypeA(AsmParser &parser,
-                                      FailureOr<int> &aResult) {
-  aResult.emplace();
-  return parser.parseInteger(*aResult);
+static LogicalResult parseCustomTypeA(AsmParser &parser, int &aResult) {
+  return parser.parseInteger(aResult);
 }
 
 static void printCustomTypeA(AsmPrinter &printer, int a) { printer << a; }
 
 static LogicalResult parseCustomTypeB(AsmParser &parser, int a,
-                                      FailureOr<std::optional<int>> &bResult) {
+                                      std::optional<int> &bResult) {
   if (a < 0)
     return success();
   for (int i : llvm::seq(0, a))
     if (failed(parser.parseInteger(i)))
       return failure();
   bResult.emplace(0);
-  return parser.parseInteger(**bResult);
+  return parser.parseInteger(*bResult);
 }
 
 static void printCustomTypeB(AsmPrinter &printer, int a, std::optional<int> b) {
   if (a < 0)
     return;
   printer << ' ';
   for (int i : llvm::seq(0, a))
     printer << i << ' ';
   printer << *b;
 }
 
-static LogicalResult parseFooString(AsmParser &parser,
-                                    FailureOr<std::string> &foo) {
+static LogicalResult parseFooString(AsmParser &parser, std::string &foo) {
   std::string result;
   if (parser.parseString(&result))
     return failure();
   foo = std::move(result);
   return success();
 }
 
 static void printFooString(AsmPrinter &printer, StringRef foo) {

mlir/test/mlir-tblgen/attr-or-type-format.td

 def TypeK : TestType<"TestM"> {
   let parameters = (ins ConvertFromStorageParameter:$a);
   let mnemonic = "type_k";
   let assemblyFormat = "$a";
 }
 
 // TYPE-LABEL: ::mlir::Type TestNType::parse
 // TYPE: parseFoo(
-// TYPE-NEXT: _result_a,
+// TYPE-NEXT: ::mlir::detail::unwrapForCustomParse(_result_a),
 // TYPE-NEXT: 1);
 
 // TYPE-LABEL: void TestNType::print
 // TYPE: printFoo(
 // TYPE-NEXT: getA(),
 // TYPE-NEXT: 1);
 
 def TypeL : TestType<"TestN"> {

mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp

void DefFormat::genCustomParser(CustomDirective *el, FmtContext &ctx,
   // returns failure or if any of the required parameters failed.
   os << tgfmt("auto odsCustomLoc = $_parser.getCurrentLocation();\n", &ctx);
   os << "(void)odsCustomLoc;\n";
   os << tgfmt("auto odsCustomResult = parse$0($_parser", &ctx, el->getName());
   os.indent();
   for (FormatElement *arg : el->getArguments()) {
     os << ",\n";
     if (auto *param = dyn_cast<ParameterElement>(arg))
-      os << "_result_" << param->getName();
+      os << "::mlir::detail::unwrapForCustomParse(_result_" << param->getName()
+         << ")";
     else if (auto *ref = dyn_cast<RefDirective>(arg))
       os << "*_result_" << cast<ParameterElement>(ref->getArg())->getName();
     else
       os << tgfmt(cast<StringElement>(arg)->getValue(), &ctx);
   }
   os.unindent() << ");\n";
   os << "if (::mlir::failed(odsCustomResult)) return {};\n";
   for (FormatElement *arg : el->getArguments()) {