Fix closure untupling/retupling#373
Fix closure untupling/retupling#373avanhatt merged 4 commits intomodel-checking:main-154-2021-08-02from
Conversation
433c9c6 to
c6025d5
Compare
|
Hi @avanhatt , can you please add the example to the description? |
adpaco-aws
left a comment
There was a problem hiding this comment.
Great work! A few minor comments and questions.
There was a problem hiding this comment.
Why do we need this check?
There was a problem hiding this comment.
The spread arg is added in the prelude, so we shouldn't re-add it to the table here. But I added a different, more explicit check and comment instead, thanks!
There was a problem hiding this comment.
Do we need to hard-code this?
There was a problem hiding this comment.
I think so, the name spread is not special, we just need to name it something different from var and I chose this to try and help convey the purpose.
99e693b to
f3aa6c7
Compare
adpaco-aws
left a comment
There was a problem hiding this comment.
LGTM! Do you mind updating manually and fixing the typo below?
Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Use the rust-call ABI and MIR-level spread args flag to untuple/retuple closures and shims. Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Use the rust-call ABI and MIR-level spread args flag to untuple/retuple closures and shims. Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Use the rust-call ABI and MIR-level spread args flag to untuple/retuple closures and shims. Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Use the rust-call ABI and MIR-level spread args flag to untuple/retuple closures and shims. Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Use the rust-call ABI and MIR-level spread args flag to untuple/retuple closures and shims. Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Description of changes:
RMC currently fails to verify some nested boxed closures, such as this:
The problem is that we were checking whether to untuple closure arguments based on whether the function's type kind was a
Closure, but this misses some cases (where the trait points to a defined function, or with vtable shims) where the type is aFnDef. The right way to check for untupling is via a check to the function's ABI. However, in that case, we also need to retuple arguments based on the MIR'sspread_argflag. See this Zulip for more discussion.The trickiness here is that we need to insert both the tuple and its individual components into the symbol table, and then write the components out to the tuple at the start of a function. To do this, I added a new naming convention for the untupled args, avoiding naming conflicts based on the local index. Local index math determines what gets retupled when necessary.
Here is an example of the code we now generate (with renaming for clarity):
Resolved issues:
Resolves #240
Call-outs:
ignore_var_tyis mystifying, and there are some intrinsic cases where skippingFnDefthere elides the environment argument of a call through a tuple. I don't like this, and we should probably eventually removeignore_var_tyin favor of elides arguments only for specific, documented reasons (like the being Zero Sized Types that rust elides).Testing:
Existing tests, including a
fixmeadded in my last PR.Somewhat.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.