Page MenuHomePhabricator

[mlir] Add an assertion on creating an Operation with null result types
ClosedPublic

Authored by ftynse on Nov 19 2020, 1:55 AM.

Details

Summary

Null types are commonly used as an error marker. Catch them in the constructor
of Operation if they are present in the result type list, as otherwise this
could lead to further surprising behavior when querying op result types.

Fix AsyncToLLVM and StandardToLLVM that were using null types when constructing
operations.

Diff Detail

Event Timeline

ftynse created this revision.Nov 19 2020, 1:55 AM
ftynse requested review of this revision.Nov 19 2020, 1:55 AM
ftynse added inline comments.Nov 19 2020, 1:58 AM
mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
226

We could also consider adding a TypeRange constructor that creates an empty range out of a null type, but that would look surprising to me in the case of

Type t = getTypeOrNull();
something(TypeRange(t)); // empty range not really expected
rriddle accepted this revision.Nov 19 2020, 9:56 AM

Thanks for the quick fix!

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
226

I was actually thinking the opposite, i.e. assert that all provided types(from an ArrayRef) to a TypeRange are non-null.

This revision is now accepted and ready to land.Nov 19 2020, 9:56 AM
ftynse updated this revision to Diff 306474.Nov 19 2020, 10:35 AM

Add a check in TypeRange

mlir/lib/Conversion/AsyncToLLVM/AsyncToLLVM.cpp
226

Added this, too.