Fix bug for Ops with default valued attributes and successors/variadi…

…c regions.

When both a DefaultValuedAttr and a successor or variadic region was specified, this would generate invalid C++ declaration. There would be the parameter with a default value, followed by the successors/regions, which don't have a default, which is invalid.

Reviewed By: mehdi_amini

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

mlir/test/mlir-tblgen/op-attribute.td

@@ -278,6 +278,30 @@ def MixOperandsAndAttrs : NS_Op<"mix_operands_and_attrs", []> {
   let arguments = (ins F32Attr:$attr, F32:$operand, F32Attr:$otherAttr, F32:$otherArg);
 }
 
+def OpWithDefaultAndRegion : NS_Op<"default_with_region", []> {
+  let arguments = (ins
+          DefaultValuedAttr<BoolAttr, "true">:$dv_bool_attr
+  );
+  let regions = (region VariadicRegion<AnyRegion>:$region);
+}
+
+// We should not have a default attribute in this case.
+
+// DECL-LABEL: OpWithDefaultAndRegion declarations
+// DECL: static void build({{.*}}, bool dv_bool_attr, unsigned regionCount)
+
+def OpWithDefaultAndSuccessor : NS_Op<"default_with_succ", []> {
+  let arguments = (ins
+          DefaultValuedAttr<BoolAttr, "true">:$dv_bool_attr
+  );
+  let successors = (successor VariadicSuccessor<AnySuccessor>:$succ);
+}
+
+// We should not have a default attribute in this case.
+
+// DECL-LABEL: OpWithDefaultAndSuccessor declarations
+// DECL: static void build({{.*}}, bool dv_bool_attr, ::mlir::BlockRange succ)
+
 // DEF-LABEL: MixOperandsAndAttrs definitions
 // DEF-DAG: ::mlir::Value MixOperandsAndAttrs::operand()
 // DEF-DAG: ::mlir::Value MixOperandsAndAttrs::otherArg()

mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp

@@ -1512,7 +1512,10 @@ void OpEmitter::buildParamList(SmallVectorImpl<OpMethodParameter> &paramList,
 
   // Add parameters for all arguments (operands and attributes).
   int defaultValuedAttrStartIndex = op.getNumArgs();
-  if (attrParamKind == AttrParamKind::UnwrappedValue) {
+  // Successors and variadic regions go at the end of the parameter list, so no
+  // default arguments are possible.
+  bool hasTrailingParams = op.getNumSuccessors() || op.getNumVariadicRegions();
+  if (attrParamKind == AttrParamKind::UnwrappedValue && !hasTrailingParams) {
     // Calculate the start index from which we can attach default values in the
     // builder declaration.
     for (int i = op.getNumArgs() - 1; i >= 0; --i) {