[AsmPrinter] Make OpAsmPrinter::printFunctionalType be resilient to null values.

[lattner]

Summary

A previous patch made Value::getType() be resilient to null values which was
considered to be too sweeping. This is a more targeted change which requires
deabstracting some templates.

A middle ground would be to make ValueTypeIterator be tolerant to null values.

[jurahul]

mlir/lib/IR/AsmPrinter.cpp

60

Should this use a different variable name for the "Value op" argument to the lambda since there exists "Operation *op" as well? I was initially confused due to the name reuse here.

[rriddle]

mlir/lib/IR/AsmPrinter.cpp

68

nit: Spell out auto here or just inline the op->getNumResults() into the check below.

[rriddle]

Thanks!

mlir/include/mlir/IR/OpImplementation.h

void printArrowTypeList(TypeRange &&types) {
     if (wrapped)
       os << '(';
     llvm::interleaveComma(types, *this);
     if (wrapped)
       os << ')';
   }
 
   /// Print the complete type of an operation in functional form.
-  void printFunctionalType(Operation *op) {
-    printFunctionalType(op->getOperandTypes(), op->getResultTypes());
-  }
+  void printFunctionalType(Operation *op);
+
   /// Print the two given type ranges in a functional form.
   template <typename InputRangeT, typename ResultRangeT>
   void printFunctionalType(InputRangeT &&inputs, ResultRangeT &&results) {
     auto &os = getStream();
-    os << "(";
+    os << '(';
     llvm::interleaveComma(inputs, *this);
-    os << ")";
+    os << ')';
     printArrowTypeList(results);
   }
 
   /// Print the given string as a symbol reference, i.e. a form representable by
   /// a SymbolRefAttr. A symbol reference is represented as a string prefixed
   /// with '@'. The reference is surrounded with ""'s and escaped if it has any
   /// special or non-printable characters in it.
   virtual void printSymbolName(StringRef symbolRef) = 0;
public:
 
   /// Parse a `]` token if present.
   virtual ParseResult parseOptionalRSquare() = 0;
 
   /// Parse a `...` token if present;
   virtual ParseResult parseOptionalEllipsis() = 0;
 
   /// Parse an integer value from the stream.
-  template <typename IntT> ParseResult parseInteger(IntT &result) {
+  template <typename IntT>
+  ParseResult parseInteger(IntT &result) {
     auto loc = getCurrentLocation();
     OptionalParseResult parseResult = parseOptionalInteger(result);
     if (!parseResult.hasValue())
       return emitError(loc, "expected integer value");
     return *parseResult;
   }
 
   /// Parse an optional integer value from the stream.

mlir/lib/IR/AsmPrinter.cpp

 void Identifier::dump() const { print(llvm::errs()); }
 
 void OperationName::print(raw_ostream &os) const { os << getStringRef(); }
 
 void OperationName::dump() const { print(llvm::errs()); }
 
 DialectAsmPrinter::~DialectAsmPrinter() {}
 
+//===--------------------------------------------------------------------===//
+// OpAsmPrinter
+//===--------------------------------------------------------------------===//
+
 OpAsmPrinter::~OpAsmPrinter() {}
 
+void OpAsmPrinter::printFunctionalType(Operation *op) {
+  auto &os = getStream();
+  os << '(';
+  llvm::interleaveComma(op->getOperands(), os, [&](Value op) {
+    // Print the types of null values as <<NULL TYPE>>.
+    *this << (op ? op.getType() : Type());
+  });
+  os << ") -> ";
+
+  // Print the result list.  We don't parenthesize single result types unless
+  // it is a function (avoiding a grammar ambiguity).
+  auto numResults = op->getNumResults();
+  bool wrapped = numResults != 1;
+  if (!wrapped && op->getResult(0).getType() &&
+      op->getResult(0).getType().isa<FunctionType>())
+    wrapped = true;
+
+  if (wrapped)
+    os << '(';
+
+  llvm::interleaveComma(op->getResults(), os, [&](const OpResult &r) {
+    // Print the types of null values as <<NULL TYPE>>.
+    *this << (r ? r.getType() : Type());
+  });
+
+  if (wrapped)
+    os << ')';
+}
+
 //===--------------------------------------------------------------------===//
 // Operation OpAsm interface.
 //===--------------------------------------------------------------------===//
 
 /// The OpAsmOpInterface, see OpAsmInterface.td for more details.
 #include "mlir/IR/OpAsmInterface.cpp.inc"
 
 //===----------------------------------------------------------------------===//

mlir/lib/IR/Value.cpp

Value::Value(Operation *op, unsigned resultNo) {
 
   // If we can't pack the result directly, grab the use list from the parent op.
   unsigned trailingNo = resultNo - OpResult::getMaxInlineResults();
   ownerAndKind = {op->getTrailingResult(trailingNo), Kind::TrailingOpResult};
 }
 
 /// Return the type of this value.
 Type Value::getType() const {
-  // Support a null Value so the asmprinter doesn't crash on invalid IR (e.g.
-  // operations that have dropAllReferences() called on them).
-  if (!*this)
-    return Type();
-
   if (BlockArgument arg = dyn_cast<BlockArgument>())
     return arg.getType();
 
   // If this is an operation result, query the parent operation.
   OpResult result = cast<OpResult>();
   Operation *owner = result.getOwner();
   if (owner->hasSingleResult)
     return owner->resultType;