diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h --- a/mlir/include/mlir/IR/OpImplementation.h +++ b/mlir/include/mlir/IR/OpImplementation.h @@ -645,30 +645,34 @@ // Region Parsing //===--------------------------------------------------------------------===// - /// Parses a region. Any parsed blocks are appended to "region" and must be + /// Parses a region. Any parsed blocks are appended to 'region' and must be /// moved to the op regions after the op is created. The first block of the - /// region takes "arguments" of types "argTypes". If "enableNameShadowing" is + /// region takes 'arguments' of types 'argTypes'. If 'enableNameShadowing' is /// set to true, the argument names are allowed to shadow the names of other - /// existing SSA values defined above the region scope. "enableNameShadowing" + /// existing SSA values defined above the region scope. 'enableNameShadowing' /// can only be set to true for regions attached to operations that are - /// "IsolatedFromAbove". + /// 'IsolatedFromAbove. If 'allowEmptyRegion' is false, parsing will fail if + /// the region is empty. virtual ParseResult parseRegion(Region ®ion, ArrayRef arguments = {}, ArrayRef argTypes = {}, - bool enableNameShadowing = false) = 0; + bool enableNameShadowing = false, + bool allowEmptyRegion = true) = 0; /// Parses a region if present. virtual ParseResult parseOptionalRegion(Region ®ion, ArrayRef arguments = {}, ArrayRef argTypes = {}, - bool enableNameShadowing = false) = 0; + bool enableNameShadowing = false, + bool allowEmptyRegion = true) = 0; /// Parses a region if present. If the region is present, a new region is /// allocated and placed in `region`. If no region is present or on failure, /// `region` remains untouched. virtual OptionalParseResult parseOptionalRegion( std::unique_ptr ®ion, ArrayRef arguments = {}, - ArrayRef argTypes = {}, bool enableNameShadowing = false) = 0; + ArrayRef argTypes = {}, bool enableNameShadowing = false, + bool allowEmptyRegion = true) = 0; /// Parse a region argument, this argument is resolved when calling /// 'parseRegion'. diff --git a/mlir/lib/IR/FunctionImplementation.cpp b/mlir/lib/IR/FunctionImplementation.cpp --- a/mlir/lib/IR/FunctionImplementation.cpp +++ b/mlir/lib/IR/FunctionImplementation.cpp @@ -204,10 +204,13 @@ assert(resultAttrs.size() == resultTypes.size()); addArgAndResultAttrs(builder, result, argAttrs, resultAttrs); - // Parse the optional function body. + // Parse the optional function body. The printer will not print the body if + // its empty, so disallow parsing of empty body in the parser. auto *body = result.addRegion(); return parser.parseOptionalRegion( - *body, entryArgs, entryArgs.empty() ? ArrayRef() : argTypes); + *body, entryArgs, entryArgs.empty() ? ArrayRef() : argTypes, + /*enableNameShadowing=*/false, + /*allowEmptyRegion=*/false); } // Print a function result list. diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp --- a/mlir/lib/Parser/Parser.cpp +++ b/mlir/lib/Parser/Parser.cpp @@ -203,10 +203,12 @@ /// Parse a region into 'region' with the provided entry block arguments. /// 'isIsolatedNameScope' indicates if the naming scope of this region is - /// isolated from those above. + /// isolated from those above. If 'allowEmptyRegion' is false, parsing will + /// fail if the region is empty. ParseResult parseRegion(Region ®ion, ArrayRef> entryArguments, - bool isIsolatedNameScope = false); + bool isIsolatedNameScope = false, + bool allowEmptyRegion = true); /// Parse a region body into 'region'. ParseResult parseRegionBody(Region ®ion); @@ -1404,8 +1406,8 @@ /// Parse a region that takes `arguments` of `argTypes` types. This /// effectively defines the SSA values of `arguments` and assigns their type. ParseResult parseRegion(Region ®ion, ArrayRef arguments, - ArrayRef argTypes, - bool enableNameShadowing) override { + ArrayRef argTypes, bool enableNameShadowing, + bool allowEmptyRegion) override { assert(arguments.size() == argTypes.size() && "mismatching number of arguments and types"); @@ -1423,7 +1425,8 @@ assert((!enableNameShadowing || opDefinition->hasProperty(OperationProperty::IsolatedFromAbove)) && "name shadowing is only allowed on isolated regions"); - if (parser.parseRegion(region, regionArguments, enableNameShadowing)) + if (parser.parseRegion(region, regionArguments, enableNameShadowing, + allowEmptyRegion)) return failure(); return success(); } @@ -1432,23 +1435,27 @@ ParseResult parseOptionalRegion(Region ®ion, ArrayRef arguments, ArrayRef argTypes, - bool enableNameShadowing) override { + bool enableNameShadowing, + bool allowEmptyRegion) override { if (parser.getToken().isNot(Token::l_brace)) return success(); - return parseRegion(region, arguments, argTypes, enableNameShadowing); + return parseRegion(region, arguments, argTypes, enableNameShadowing, + allowEmptyRegion); } /// Parses a region if present. If the region is present, a new region is /// allocated and placed in `region`. If no region is present, `region` /// remains untouched. - OptionalParseResult - parseOptionalRegion(std::unique_ptr ®ion, - ArrayRef arguments, ArrayRef argTypes, - bool enableNameShadowing = false) override { + OptionalParseResult parseOptionalRegion(std::unique_ptr ®ion, + ArrayRef arguments, + ArrayRef argTypes, + bool enableNameShadowing, + bool allowEmptyRegion) override { if (parser.getToken().isNot(Token::l_brace)) return llvm::None; std::unique_ptr newRegion = std::make_unique(); - if (parseRegion(*newRegion, arguments, argTypes, enableNameShadowing)) + if (parseRegion(*newRegion, arguments, argTypes, enableNameShadowing, + allowEmptyRegion)) return failure(); region = std::move(newRegion); @@ -1712,14 +1719,19 @@ ParseResult OperationParser::parseRegion( Region ®ion, ArrayRef> entryArguments, - bool isIsolatedNameScope) { + bool isIsolatedNameScope, bool allowEmptyRegion) { + llvm::SMLoc regionLoc = getToken().getLoc(); + // Parse the '{'. if (parseToken(Token::l_brace, "expected '{' to begin a region")) return failure(); // Check for an empty region. - if (entryArguments.empty() && consumeIf(Token::r_brace)) - return success(); + if (entryArguments.empty() && consumeIf(Token::r_brace)) { + if (allowEmptyRegion) + return success(); + return emitError(regionLoc, "expected non-empty region"); + } auto currentPt = opBuilder.saveInsertionPoint(); // Push a new named value scope. diff --git a/mlir/test/Dialect/SCF/canonicalize.mlir b/mlir/test/Dialect/SCF/canonicalize.mlir --- a/mlir/test/Dialect/SCF/canonicalize.mlir +++ b/mlir/test/Dialect/SCF/canonicalize.mlir @@ -114,7 +114,7 @@ // ----- -func private @side_effect() {} +func private @side_effect() func @all_unused() { %c0 = constant 0 : index %c1 = constant 1 : index diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir --- a/mlir/test/IR/invalid.mlir +++ b/mlir/test/IR/invalid.mlir @@ -1572,3 +1572,7 @@ } return } + +// ----- + +func @foo() {} // expected-error {{expected non-empty region}} diff --git a/mlir/test/IR/traits.mlir b/mlir/test/IR/traits.mlir --- a/mlir/test/IR/traits.mlir +++ b/mlir/test/IR/traits.mlir @@ -344,12 +344,10 @@ // Test that operation with the SymbolTable Trait define a new symbol scope. "test.symbol_scope"() ({ - func private @foo() { - } + func private @foo() "test.finish" () : () -> () }) : () -> () -func private @foo() { -} +func private @foo() // -----