Fix monomorphization of unboxed closures #18144
Conversation
|
r? @pcwalton |
|
Looks fine to me but I should punt this over to @nikomatsakis. Does this look good to you? |
bkoropoff
force-pushed
the
mighty-monomorphizin-unboxed-closures
branch
from
b9f89bf
to
e16c8b7
Compare
|
Rebased |
|
Sorry for delay. Was out last week. Will look at it ASAP. |
|
This is cool. At first when I read the patch I thought this was the wrong approach, and that it made more sense to just apply the current monomorphization substitutions in scope during trans. But of course that doesn't work when you are invoking other functions where a type parameter is unbound to an unbound function type, nor will it work in future with things like One thing I would like to see is a comment explaining how substitutions in unboxed closures work. I am not sure of the best place, but the def'n of the variant
|
|
r+ modulo nits. |
|
I am unsure if I found a related or unrelated issue, but I applied this PR to work around the issue that it is fixing and then hit another ICE. See issue #18378 |
This adds a `Substs` field to `ty_unboxed_closure` and plumbs basic handling of it throughout the compiler. trans now correctly monomorphizes captured free variables and llvm function defs. This fixes uses of unboxed closures which reference a free type or region parameter from their environment in either their signature or free variables. Closes rust-lang#16791
bkoropoff
force-pushed
the
mighty-monomorphizin-unboxed-closures
branch
from
e16c8b7
to
e46af8c
Compare
|
Fixed nits and rebased on latest master due to merge conflict. I'm still running tests locally, but this should be ready for r+. |
…osures, r=nikomatsakis This allows unboxed closures that reference free type/region parameters to be monomorphized correctly in trans. It was necessary to make `ty_unboxed_closure` carry around a `Substs` to accomplish this. Plumbing this through typeck revealed several areas where type/region parameters in unboxed closure types are possibly not being handled correctly. Since my goal was just to fix trans, I decided to leave FIXME comments on areas that still need attention and seek feedback on the best way to clean them up, possibly as a follow-up PR. Closes #16791
fix: Remove check that text of `parse_expr_from_str()` matches the produced parsed tree This check is incorrect when we have comments and whitespace in the text. We can strip comments, but then we still have whitespace, which we cannot strip without changing meaning for the parser. So instead I opt to remove the check, and wrap the expression in parentheses (asserting what produced is a parenthesized expression) to strengthen verification. Fixes rust-lang#18144.
This allows unboxed closures that reference free type/region parameters to be monomorphized correctly in trans.
It was necessary to make
ty_unboxed_closurecarry around aSubststo accomplish this. Plumbing this through typeck revealed several areas where type/region parameters in unboxed closure types are possibly not being handled correctly. Since my goal was just to fix trans, I decided to leave FIXME comments on areas that still need attention and seek feedback on the best way to clean them up, possibly as a follow-up PR.Closes #16791