This is an archive of the discontinued LLVM Phabricator instance.

Remove error-prone mlir::ExecutionEngine::invoke overload.
ClosedPublic

Authored by silvas on May 26 2020, 8:14 PM.

Details

Summary

I just spent a bunch of time debugging a mysterious bug that ended being due to my SmallVector getting passed to the Args&... overload instead of the MutableArrayRef overload, with disastrous results.

I appreciate the intent of this API, but for a function that does a bunch of unsafe casts, adding in potential overload confusion is just too much C++ footgun. If we end up needing this functionality, having something like a separate packArgs(Args&...) -> SmallVector overload would be preferable.

Turns out this API is unused and untested (even out of tree as far as I can tell, modulo the optional passing of no args to the other invoke as I fixed in this patch), so it's an easy fix -- just delete it and touch up the other overload.

Diff Detail

Event Timeline

silvas created this revision.May 26 2020, 8:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2020, 8:14 PM
ftynse accepted this revision.May 27 2020, 7:18 AM
This revision is now accepted and ready to land.May 27 2020, 7:18 AM
This revision was automatically updated to reflect the committed changes.