Translate closures through the collector #37675
Conversation
arielb1
force-pushed
the
trans-closure
branch
from
f894b76
to
a759b47
Compare
| impl<'a, 'gcx, 'acx, 'tcx> ClosureSubsts<'tcx> { | ||
| #[inline] | ||
| pub fn upvar_tys(self, def_id: DefId, tcx: TyCtxt<'a, 'gcx, 'acx>) -> | ||
| iter::Map<slice::Iter<'tcx, Kind<'tcx>>, KindAsType> |
There was a problem hiding this comment.
Should just use impl Trait here, maybe you can share the error?
Last I checked most cases can be tricked into inferring correctly.
There was a problem hiding this comment.
error[E0564]: only named lifetimes are allowed in `impl Trait`, but `` was found in the type `core::iter::Map<core::slice::Iter<'_, ty::subst::Kind<'tcx>>, [closure@../src/librustc/ty/sty.rs:292:13: 292:66]>`
| @@ -254,15 +257,38 @@ pub enum TypeVariants<'tcx> { | |||
| /// handle). Plus it fixes an ICE. :P | |||
| #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)] | |||
| pub struct ClosureSubsts<'tcx> { | |||
| /// Lifetime and type parameters from the enclosing function. | |||
| /// Lifetime and type parameters from the enclosing function, | |||
| /// concatenated with the types of the upvars. | |||
There was a problem hiding this comment.
This is pretty much my only concern, that the upvar types are here - in fact, why are they in the type at all?
Why can't they be like ADTs, with the closure pointing to a list of "fields" which are inferred by typeck?
There was a problem hiding this comment.
Before trans, closure types need the upvars because they contain lifetimes.
There was a problem hiding this comment.
But couldn't the types with those lifetimes be stored somewhere else, keyed by DefId?
I believe we already do something like that for the signature of the closure.
arielb1
force-pushed
the
trans-closure
branch
from
a759b47
to
28b2ad2
Compare
|
cc @rust-lang/compiler |
|
Nice |
arielb1
force-pushed
the
trans-closure
branch
from
28b2ad2
to
8701725
Compare
alexcrichton
added
the
T-compiler
|
☔ The latest upstream changes (presumably #37730) made this pull request unmergeable. Please resolve the merge conflicts. |
This moves closures to the (DefId, Substs) scheme like all other items, and saves a word from the size of TyS now that Substs is 2 words.
This makes them closer to actual items and allows for more transparent treatment.
Translate closures like normal functions, using the trans::collector interface.
arielb1
force-pushed
the
trans-closure
branch
from
8701725
to
5795098
Compare
| (base_generics.types.len() as u32), | ||
| regions: vec![], | ||
| types: upvar_decls, | ||
| has_self: false, |
There was a problem hiding this comment.
This should be base_generics.has_self. Nevertheless, putting generics/type here is a problem.
In #37676 I've added generics for closures in typeck::collect, which is closer to what on-demand type-checking needs.
OTOH your closure_base_def_id is better (or even actually correct) than what I have there, so I'd prefer it.
|
@arielb1 r=me with the closure generics/type creation moved to |
|
@bors r+ |
|
📌 Commit d394e75 has been approved by |
Translate closures through the collector Now that old trans is gone, there is no need for the hack of translating closures when they are instantiated. We can translate them like regular items. r? @eddyb
Now that old trans is gone, there is no need for the hack of translating closures when they are instantiated. We can translate them like regular items.
r? @eddyb