Page MenuHomePhabricator

[MLIR] Fix parseFunctionLikeOp() to fail parsing empty regions
ClosedPublic

Authored by jurahul on Nov 20 2020, 11:51 AM.

Details

Summary

Diff Detail

Event Timeline

jurahul created this revision.Nov 20 2020, 11:51 AM
jurahul requested review of this revision.Nov 20 2020, 11:51 AM
mehdi_amini accepted this revision.Nov 24 2020, 3:31 PM
This revision is now accepted and ready to land.Nov 24 2020, 3:31 PM

Can't the users of parseRegion just check to see if it is empty after success and emit an error if they don't want that behavior?

Not really. Note that both

func @foo()
func @foo() {}

post parsing look the same (the region attached to FuncOp is empty). So that's too late for the check.

mlir/lib/IR/FunctionImplementation.cpp
209–210

For reference, we cannot do the error check here after parser.parseOptionalRegion() because there is no way to distinguish between no region in the assembly (func @foo) vs an empty region (func @foo {}).

rriddle added inline comments.Dec 2 2020, 10:54 AM
mlir/lib/IR/FunctionImplementation.cpp
209–210

OptionalParseResult represents 3 different value states. Just change the other parseOptionalRegion to use OptionalParseResult (which it should be doing anyways). Then you could do the following:

OptionalParseResult result = parser.parseOptionalRegion(region);
if (result) {
  if (failed(*result))
    return failure();
  if (region.empty())
    ...
}
return success();
jurahul updated this revision to Diff 309104.Dec 2 2020, 4:50 PM

River's feedback

rriddle accepted this revision.Dec 3 2020, 3:36 PM

Nice!

This revision was landed with ongoing or failed builds.Dec 4 2020, 9:10 AM
This revision was automatically updated to reflect the committed changes.