Use tuple inference for closures #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arora-aman
reviewed
Sep 17, 2020
| for upvar_ty in substs.as_closure().upvar_tys() { | ||
| compute_components(tcx, upvar_ty, out); | ||
| let tupled_ty = substs.as_closure().tupled_upvars_ty(); | ||
| if matches!(tupled_ty.kind(), ty::Tuple(_)) { |
There was a problem hiding this comment.
This is my bad, this can just be substs.as_closure().is_valid()
arora-aman
reviewed
Sep 28, 2020
| @@ -110,7 +110,12 @@ pub fn trivial_dropck_outlives<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { | |||
| // check if *any* of those are trivial. | |||
| ty::Tuple(ref tys) => tys.iter().all(|t| trivial_dropck_outlives(tcx, t.expect_ty())), | |||
| ty::Closure(_, ref substs) => { | |||
| substs.as_closure().upvar_tys().all(|t| trivial_dropck_outlives(tcx, t)) | |||
| if !substs.as_closure().is_valid() { | |||
There was a problem hiding this comment.
This can just be trivial_dropck_outlives(tcx, substs.as_closure().tupled_upvar_tys())
| }) | ||
| })); | ||
| let tupled_upvars_ty = self.infcx.next_ty_var(TypeVariableOrigin { | ||
| // FIXME(eddyb) distinguish upvar inference variables from the rest. |
There was a problem hiding this comment.
I feel like this isn't valid anymore, individual inference variable for each upvar, isn't a thing anymore.
roxelo
force-pushed
the
use_tuple_inference_for_closures
branch
from
fd4c7e3 to
30ab719
Compare
roxelo
commented
Oct 4, 2020
| @@ -1307,6 +1307,26 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | |||
| let mut generator = None; | |||
| let mut outer_generator = None; | |||
| let mut next_code = Some(&obligation.cause.code); | |||
| // By introducing a tuple into the upvar types, the first item in the obligation chain | |||
| // can be of Tuple type instead of the generator that captured the type. We want to catch for | |||
| // this case and skip it. | |||
There was a problem hiding this comment.
This does the trick for now, but it's probably not what we want to move forward with. The tuple of upvar type should not be the first item in the obligation chain, probably more in the middle. We probably want to change how the obligation chain is created. I haven't figured out where in the code that is done yet though
roxelo
force-pushed
the
use_tuple_inference_for_closures
branch
from
fe5b9c1 to
5648be6
Compare
arora-aman
force-pushed
the
use_tuple_inference_for_closures
branch
3 times, most recently
from
aa27996 to
66d5d96
Compare
arora-aman
force-pushed
the
use_tuple_inference_for_closures
branch
2 times, most recently
from
ee267c0 to
0bd6588
Compare
This commit allows us to decide the number of captures required after completing capture ananysis, which is required as part of implementing RFC-2229. Co-authored-by: Aman Arora <me@aman-arora.com> Co-authored-by: Jenny Wills <wills.jenniferg@gmail.com>
Depending on if upvar_tys inferred or not, we were returning either an inference variable which later resolves to a tuple or else the upvar tys themselves Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
arora-aman
force-pushed
the
use_tuple_inference_for_closures
branch
from
0bd6588 to
3c46fd6
Compare
ChrisPardy
approved these changes
Oct 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main motivation behind this change is to allow the compiler to decide the number of captures after capture analysis instead of before capture analysis. T
This is required to support RFC-2229, since the number of captures will not be know before capture analysis.
Closes rust-lang/project-rfc-2229#4
This change is