Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow individual upvars to be inferred as by-value or by-ref in non-escaping closures #21604

Merged
merged 4 commits into from Jan 30, 2015

Conversation

Projects
None yet
5 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jan 24, 2015

r? @eddyb

@@ -100,8 +100,7 @@ fn load_unboxed_closure_environment<'blk, 'tcx>(
#[derive(PartialEq)]
pub enum ClosureKind<'tcx> {

This comment has been minimized.

@eddyb

eddyb Jan 24, 2015

Member

'tcx is not used here anymore.

This comment has been minimized.

@eddyb

eddyb Jan 24, 2015

Member

On a second note, ClosureEnv can now become Closure(&'a [ty::Freevar]) | NotClosure, which seems more idiomatic.

@@ -304,7 +304,7 @@ impl<'a, 'tcx> mc::Typer<'tcx> for FnCtxt<'a, 'tcx> {
}
fn type_moves_by_default(&self, span: Span, ty: Ty<'tcx>) -> bool {
let ty = self.infcx().resolve_type_vars_if_possible(&ty);
traits::type_known_to_meet_builtin_bound(self.infcx(), self, ty, ty::BoundCopy, span)
!traits::type_known_to_meet_builtin_bound(self.infcx(), self, ty, ty::BoundCopy, span)

This comment has been minimized.

@eddyb

eddyb Jan 24, 2015

Member

Uhm... what? How did this remain unnoticed until now?

This comment has been minimized.

@nikomatsakis

nikomatsakis Jan 26, 2015

Author Contributor

This code path isn't used except for upvar inference.

#[derive(PartialEq, Clone, RustcEncodable, RustcDecodable, Show, Copy)]
pub struct UpvarBorrow {
/// The kind of borrow: by-ref upvars have access to shared
/// immutable borrows, which are not part of the normal language

This comment has been minimized.

@eddyb

eddyb Jan 24, 2015

Member

"Shared immutable borrows" sounds like &T - did you mean to refer to &uniq T instead?

This comment has been minimized.

@nikomatsakis

nikomatsakis Jan 26, 2015

Author Contributor

Yes, thanks.

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jan 24, 2015

r=me with nits fixed. There seems to be partial overlap with #21598, but I don't think it will be an issue.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 25, 2015

\o/

@nikomatsakis nikomatsakis force-pushed the nikomatsakis:closure-move-indiv-vars branch from 8ec5d3a to 4bfba7e Jan 27, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 27, 2015

@bors r=eddyb 4bfba7e

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jan 27, 2015

@nikomatsakis did you mean to include @kvark's commit into this?

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jan 27, 2015

Oh it's in master already. @bors r- until that's fixed.

@nikomatsakis nikomatsakis force-pushed the nikomatsakis:closure-move-indiv-vars branch 2 times, most recently from 617dfb5 to 5272887 Jan 27, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 27, 2015

@bors r=eddyb 5272887

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 27, 2015

@eddyb corrected

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 27, 2015

⌛️ Testing commit 5272887 with merge 6534aa5...

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 27, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jan 27, 2015

@bors: retry

@nikomatsakis nikomatsakis force-pushed the nikomatsakis:closure-move-indiv-vars branch from 5272887 to e58a960 Jan 29, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 29, 2015

@bors r=eddyb e58a960

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 29, 2015

⌛️ Testing commit e58a960 with merge 0aacf2f...

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 29, 2015

💔 Test failed - auto-linux-64-nopt-t

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 30, 2015

Grr, this test failure is very strange! Certainly I didn't see it locally.

@nikomatsakis nikomatsakis force-pushed the nikomatsakis:closure-move-indiv-vars branch from e58a960 to ced1062 Jan 30, 2015

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Jan 30, 2015

@bors r=eddyb ced1062

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 30, 2015

⌛️ Testing commit ced1062 with merge 3fbfad3...

bors added a commit that referenced this pull request Jan 30, 2015

@bors bors merged commit ced1062 into rust-lang:master Jan 30, 2015

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
homu Test successful
Details

@nikomatsakis nikomatsakis deleted the nikomatsakis:closure-move-indiv-vars branch Mar 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.