[OpAsmParser] Simplify logic for requiredOperandCount in parseOperandList.

[lattner]

Summary

I would ideally like to eliminate 'requiredOperandCount' as a bit of
verification that should be in the client side, but it is much more
widely used than I expected. Just tidy some pieces up around it given
we can't drop it immediately.

NFC.

[lattner]

thx for the review!

mlir/include/mlir/IR/OpImplementation.h

public:
 
   /// Parse a single operand if present.
   virtual OptionalParseResult
   parseOptionalOperand(UnresolvedOperand &result,
                        bool allowResultNumber = true) = 0;
 
   /// Parse zero or more SSA comma-separated operand references with a specified
   /// surrounding delimiter, and an optional required operand count.
-  virtual ParseResult parseOperandList(
-      SmallVectorImpl<UnresolvedOperand> &result, int requiredOperandCount = -1,
-      Delimiter delimiter = Delimiter::None, bool allowResultNumber = true) = 0;
+  virtual ParseResult
+  parseOperandList(SmallVectorImpl<UnresolvedOperand> &result,
+                   Delimiter delimiter = Delimiter::None,
+                   bool allowResultNumber = true,
+                   int requiredOperandCount = -1) = 0;
 
+  /// Parse a specified number of comma separated operands.
   ParseResult parseOperandList(SmallVectorImpl<UnresolvedOperand> &result,
-                               Delimiter delimiter,
-                               bool allowResultNumber = true) {
-    return parseOperandList(result, /*requiredOperandCount=*/-1, delimiter,
-                            allowResultNumber);
+                               int requiredOperandCount,
+                               Delimiter delimiter = Delimiter::None) {
+    return parseOperandList(result, delimiter,
+                            /*allowResultNumber=*/true, requiredOperandCount);
   }
 
   /// Parse zero or more trailing SSA comma-separated trailing operand
   /// references with a specified surrounding delimiter, and an optional
-  /// required operand count. A leading comma is expected before the operands.
-  virtual ParseResult
-  parseTrailingOperandList(SmallVectorImpl<UnresolvedOperand> &result,
-                           int requiredOperandCount = -1,
-                           Delimiter delimiter = Delimiter::None) = 0;
+  /// required operand count. A leading comma is expected before the
+  /// operands.
   ParseResult
   parseTrailingOperandList(SmallVectorImpl<UnresolvedOperand> &result,
-                           Delimiter delimiter) {
-    return parseTrailingOperandList(result, /*requiredOperandCount=*/-1,
-                                    delimiter);
+                           Delimiter delimiter = Delimiter::None) {
+    if (failed(parseOptionalComma()))
+      return success(); // The comma is optional.
+    return parseOperandList(result, delimiter);
   }
 
   /// Resolve an operand to an SSA value, emitting an error on failure.
   virtual ParseResult resolveOperand(const UnresolvedOperand &operand,
                                      Type type,
                                      SmallVectorImpl<Value> &result) = 0;
 
   /// Resolve a list of operands to SSA values, emitting an error on failure, or
if (!result.hasValue())
       return emitError(getCurrentLocation(), "expected '('");
     return result.getValue();
   }
 
   virtual OptionalParseResult
   parseOptionalAssignmentListWithTypes(SmallVectorImpl<UnresolvedOperand> &lhs,
                                        SmallVectorImpl<UnresolvedOperand> &rhs,
                                        SmallVectorImpl<Type> &types) = 0;
-
-private:
-  /// Parse either an operand list or a region argument list depending on
-  /// whether isOperandList is true.
-  ParseResult
-  parseOperandOrRegionArgList(SmallVectorImpl<UnresolvedOperand> &result,
-                              bool isOperandList, int requiredOperandCount,
-                              Delimiter delimiter);
 };
 
 //===--------------------------------------------------------------------===//
 // Dialect OpAsm interface.
 //===--------------------------------------------------------------------===//
 
 /// A functor used to set the name of the start of a result group of an
 /// operation. See 'getAsmResultNames' below for more details.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

if (parser.parseOperand(srcMemRefInfo) ||
                                     getDstMapAttrName(), result.attributes) ||
       parser.parseComma() || parser.parseOperand(tagMemRefInfo) ||
       parser.parseAffineMapOfSSAIds(tagMapOperands, tagMapAttr,
                                     getTagMapAttrName(), result.attributes) ||
       parser.parseComma() || parser.parseOperand(numElementsInfo))
     return failure();
 
   // Parse optional stride and elements per stride.
-  if (parser.parseTrailingOperandList(strideInfo)) {
+  if (parser.parseTrailingOperandList(strideInfo))
     return failure();
-  }
+
   if (!strideInfo.empty() && strideInfo.size() != 2) {
     return parser.emitError(parser.getNameLoc(),
                             "expected two stride related operands");
   }
   bool isStrided = strideInfo.size() == 2;
 
   if (parser.parseColonTypeList(types))
     return failure();

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

parseWsLoopControl(OpAsmParser &parser, Region &region,
   // Parse an opening `(` followed by induction variables followed by `)`
   SmallVector<OpAsmParser::UnresolvedOperand> ivs;
   if (parser.parseOperandList(ivs, OpAsmParser::Delimiter::Paren,
                               /*allowResultNumber=*/false))
     return failure();
 
   size_t numIVs = ivs.size();
   Type loopVarType;
-  if (parser.parseColonType(loopVarType))
-    return failure();
-
+  if (parser.parseColonType(loopVarType) ||
       // Parse loop bounds.
-  if (parser.parseEqual() ||
+      parser.parseEqual() ||
       parser.parseOperandList(lowerBound, numIVs,
-                              OpAsmParser::Delimiter::Paren))
-    return failure();
-  if (parser.parseKeyword("to") ||
+                              OpAsmParser::Delimiter::Paren) ||
+      parser.parseKeyword("to") ||
       parser.parseOperandList(upperBound, numIVs,
                               OpAsmParser::Delimiter::Paren))
     return failure();
 
-  if (succeeded(parser.parseOptionalKeyword("inclusive"))) {
+  if (succeeded(parser.parseOptionalKeyword("inclusive")))
     inclusive = UnitAttr::get(parser.getBuilder().getContext());
-  }
 
   // Parse step values.
   if (parser.parseKeyword("step") ||
       parser.parseOperandList(steps, numIVs, OpAsmParser::Delimiter::Paren))
     return failure();
 
   // Now parse the body.
   loopVarTypes = SmallVector<Type>(numIVs, loopVarType);

mlir/lib/Dialect/SCF/SCF.cpp

if (parser.parseKeyword("step") ||
       parser.parseOperandList(steps, ivs.size(),
                               OpAsmParser::Delimiter::Paren) ||
       parser.resolveOperands(steps, builder.getIndexType(), result.operands))
     return failure();
 
   // Parse init values.
   SmallVector<OpAsmParser::UnresolvedOperand, 4> initVals;
   if (succeeded(parser.parseOptionalKeyword("init"))) {
-    if (parser.parseOperandList(initVals, /*requiredOperandCount=*/-1,
-                                OpAsmParser::Delimiter::Paren))
+    if (parser.parseOperandList(initVals, OpAsmParser::Delimiter::Paren))
       return failure();
   }
 
   // Parse optional results in case there is a reduce.
   if (parser.parseOptionalArrowTypeList(result.types))
     return failure();
 
   // Now parse the body.

mlir/lib/Parser/Parser.cpp

parseOptionalOperand(UnresolvedOperand &result,
     if (parser.getToken().is(Token::percent_identifier))
       return parseOperand(result, allowResultNumber);
     return llvm::None;
   }
 
   /// Parse zero or more SSA comma-separated operand references with a specified
   /// surrounding delimiter, and an optional required operand count.
   ParseResult parseOperandList(SmallVectorImpl<UnresolvedOperand> &result,
-                               int requiredOperandCount = -1,
                                Delimiter delimiter = Delimiter::None,
-                               bool allowResultNumber = true) override {
+                               bool allowResultNumber = true,
+                               int requiredOperandCount = -1) override {
     auto startLoc = parser.getToken().getLoc();
 
     // The no-delimiter case has some special handling for better diagnostics.
     if (delimiter == Delimiter::None) {
       // parseCommaSeparatedList doesn't handle the missing case for "none",
       // so we handle it custom here.
       if (parser.getToken().isNot(Token::percent_identifier)) {
         // If we didn't require any operands or required exactly zero (weird)
ParseResult parseOperandList(SmallVectorImpl<UnresolvedOperand> &result,
     // Check that we got the expected # of elements.
     if (requiredOperandCount != -1 &&
         result.size() != static_cast<size_t>(requiredOperandCount))
       return emitError(startLoc, "expected ")
              << requiredOperandCount << " operands";
     return success();
   }
 
-  /// Parse zero or more trailing SSA comma-separated trailing operand
-  /// references with a specified surrounding delimiter, and an optional
-  /// required operand count. A leading comma is expected before the operands.
-  ParseResult
-  parseTrailingOperandList(SmallVectorImpl<UnresolvedOperand> &result,
-                           int requiredOperandCount,
-                           Delimiter delimiter) override {
-    if (parser.getToken().is(Token::comma)) {
-      parseComma();
-      return parseOperandList(result, requiredOperandCount, delimiter);
-    }
-    if (requiredOperandCount != -1)
-      return emitError(parser.getToken().getLoc(), "expected ")
-             << requiredOperandCount << " operands";
-    return success();
-  }
-
   /// Resolve an operand to an SSA value, emitting an error on failure.
   ParseResult resolveOperand(const UnresolvedOperand &operand, Type type,
                              SmallVectorImpl<Value> &result) override {
     if (auto value = parser.resolveSSAUse(operand, type)) {
       result.push_back(value);
       return success();
     }
     return failure();