[OpAsmParser] Simplify logic for requiredOperandCount in parseOperandList.

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.

Differential Revision: https://reviews.llvm.org/D124629

Author: lattner

mlir/include/mlir/IR/OpImplementation.h

@@ -1122,29 +1122,30 @@ class OpAsmParser : public AsmParser {
 
   /// 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.
@@ -1297,14 +1298,6 @@ class OpAsmParser : public AsmParser {
   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);
 };
 
 //===--------------------------------------------------------------------===//

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

@@ -1075,9 +1075,9 @@ ParseResult AffineDmaStartOp::parse(OpAsmParser &parser,
     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");

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

@@ -530,22 +530,18 @@ parseWsLoopControl(OpAsmParser &parser, Region &region,
 
   size_t numIVs = ivs.size();
   Type loopVarType;
-  if (parser.parseColonType(loopVarType))
-    return failure();
-
-  // Parse loop bounds.
-  if (parser.parseEqual() ||
+  if (parser.parseColonType(loopVarType) ||
+      // Parse loop bounds.
+      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") ||

mlir/lib/Dialect/SCF/SCF.cpp

@@ -2006,8 +2006,7 @@ ParseResult ParallelOp::parse(OpAsmParser &parser, OperationState &result) {
   // 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();
   }
 

mlir/lib/Parser/Parser.cpp

@@ -1297,9 +1297,9 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
   /// 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.
@@ -1339,23 +1339,6 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
     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 {