Use the named accessor methods in predicate substitutions.

[awpr]

Summary

Previously, using these substitutions for optional operands/results
would dereference a potentially-past-the-end iterator. Now, an absent
optional operand/result will correspond to a null Operand/Value, and
variadics will still correspond to ranges.

[jpienaar]

Nice, thanks. Could you add a test with an optional attr that would have failed here before?

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

private:
   FmtContext verifyCtx;
 };
 } // end anonymous namespace
 
 // Populate the format context `ctx` with substitutions of attributes, operands
 // and results.
 // - attrGet corresponds to the name of the function to call to get value of
 //   attribute (the generated function call returns an Attribute);
-// - operandGet corresponds to the name of the function with which to retrieve
-//   an operand (the generaed function call returns an OperandRange);
-// - reultGet corresponds to the name of the function to get an result (the
-//   generated function call returns a ValueRange);
+// - operands and results are retrieve via their named accessor methods; normal
+//   and optional operands/results return an Operand/Value, and variadic ones
+//   return an OperandRange/ValueRange.
 static void populateSubstitutions(const Operator &op, const char *attrGet,
-                                  const char *operandGet, const char *resultGet,
                                   FmtContext &ctx) {
   // Populate substitutions for attributes and named operands.
   for (const auto &namedAttr : op.getAttributes())
     ctx.addSubst(namedAttr.name,
                  formatv("{0}(\"{1}\")", attrGet, namedAttr.name));
   for (int i = 0, e = op.getNumOperands(); i < e; ++i) {
     auto &value = op.getOperand(i);
     if (value.name.empty())
       continue;
 
-    if (value.isVariadic())
-      ctx.addSubst(value.name, formatv("{0}({1})", operandGet, i));
-    else
-      ctx.addSubst(value.name, formatv("(*{0}({1}).begin())", operandGet, i));
+    ctx.addSubst(value.name, formatv("{0}()", value.name));
   }
 
   // Populate substitutions for results.
   for (int i = 0, e = op.getNumResults(); i < e; ++i) {
     auto &value = op.getResult(i);
     if (value.name.empty())
       continue;
 
-    if (value.isVariadic())
-      ctx.addSubst(value.name, formatv("{0}({1})", resultGet, i));
-    else
-      ctx.addSubst(value.name, formatv("(*{0}({1}).begin())", resultGet, i));
+    ctx.addSubst(value.name, formatv("{0}()", value.name));
   }
 }
 
 // Generate attribute verification. If emitVerificationRequiringOp is set then
 // only verification for attributes whose value depend on op being known are
 // emitted, else only verification that doesn't depend on the op being known are
 // generated.
 // - emitErrorPrefix is the prefix for the error emitting call which consists
void OpEmitter::genVerifier() {
   auto &body = method.body();
   body << "  if (failed(" << op.getAdaptorName()
        << "(*this).verify(this->getLoc()))) "
        << "return failure();\n";
 
   auto *valueInit = def.getValueInit("verifier");
   CodeInit *codeInit = dyn_cast<CodeInit>(valueInit);
   bool hasCustomVerify = codeInit && !codeInit->getValue().empty();
-  populateSubstitutions(op, "this->getAttr", "this->getODSOperands",
-                        "this->getODSResults", verifyCtx);
+  populateSubstitutions(op, "this->getAttr", verifyCtx);
 
   genAttributeVerifier(op, "this->getAttr", "emitOpError(",
                        /*emitVerificationRequiringOp=*/true, verifyCtx, body);
   genOperandResultVerifier(body, op.getOperands(), "operand");
   genOperandResultVerifier(body, op.getResults(), "result");
 
   for (auto &trait : op.getTraits()) {
     if (auto *t = dyn_cast<tblgen::PredOpTrait>(&trait)) {
if (auto *t = dyn_cast<tblgen::NativeOpTrait>(&trait)) {
       } else if (t->getTrait() == "OpTrait::AttrSizedResultSegments") {
         body << formatv(checkAttrSizedValueSegmentsCode, "result_segment_sizes",
                         op.getNumResults(), "result");
       }
     }
   }
 
   FmtContext verifyCtx;
-  populateSubstitutions(op, "odsAttrs.get", "getODSOperands",
-                        "<no results should be genarated>", verifyCtx);
+  populateSubstitutions(op, "odsAttrs.get", verifyCtx);
   genAttributeVerifier(op, "odsAttrs.get",
                        Twine("emitError(loc, \"'") + op.getOperationName() +
                            "' op \"",
                        /*emitVerificationRequiringOp*/ false, verifyCtx, body);
 
   body << "  return success();";
 }