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

Introduce a FreeRegionMap to store relations between free regions #24553

Merged
merged 2 commits into from
Apr 24, 2015

Conversation

nikomatsakis
Copy link
Contributor

Rather than storing the relations between free-regions in a global
table, introduce a FreeRegionMap data structure. regionck computes the
FreeRegionMap for each fn and stores the result into the tcx so that
borrowck can use it (this could perhaps be refactored to have borrowck
recompute the map, but it's a bid tedious to recompute due to the
interaction of closures and free fns). The main reason to do this is
because of #22779 -- using a global table was incorrect because when
validating impl method signatures, we want to use the free region
relationships from the trait, not the impl.

Fixes #22779.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@nikomatsakis
Copy link
Contributor Author

r? @pnkfelix

@rust-highfive rust-highfive assigned pnkfelix and unassigned brson Apr 18, 2015
table, introduce a `FreeRegionMap` data structure. regionck computes the
`FreeRegionMap` for each fn and stores the result into the tcx so that
borrowck can use it (this could perhaps be refactored to have borrowck
recompute the map, but it's a bid tedious to recompute due to the
interaction of closures and free fns). The main reason to do this is
because of rust-lang#22779 -- using a global table was incorrect because when
validating impl method signatures, we want to use the free region
relationships from the *trait*, not the impl.

Fixes rust-lang#22779.
@nikomatsakis nikomatsakis force-pushed the issue-22779-overconstrained-impl branch from 8c41bc0 to 6dfeda7 Compare April 18, 2015 15:53
@nikomatsakis nikomatsakis added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 23, 2015
@pnkfelix
Copy link
Member

soundness fix; niko considers low-risk.

going from nominated to (nominated, accepted)

(and I need to get my butt going reviewing it.)

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 23, 2015
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! This file defines
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file defines ... what? What? You leave me in suspense!

@pnkfelix
Copy link
Member

Okay, this all looks good. I had some nits about the comments but honestly I'd rather just try to land this now if possible. :)

@pnkfelix
Copy link
Member

@bors r+ 6dfeda7 p=1

@bors
Copy link
Contributor

bors commented Apr 24, 2015

⌛ Testing commit 6dfeda7 with merge af49b8f...

bors added a commit that referenced this pull request Apr 24, 2015
… r=pnkfelix

Rather than storing the relations between free-regions in a global
table, introduce a `FreeRegionMap` data structure. regionck computes the
`FreeRegionMap` for each fn and stores the result into the tcx so that
borrowck can use it (this could perhaps be refactored to have borrowck
recompute the map, but it's a bid tedious to recompute due to the
interaction of closures and free fns). The main reason to do this is
because of #22779 -- using a global table was incorrect because when
validating impl method signatures, we want to use the free region
relationships from the *trait*, not the impl.

Fixes #22779.
@bors
Copy link
Contributor

bors commented Apr 24, 2015

💔 Test failed - auto-mac-32-opt

@Manishearth
Copy link
Member

/Users/rustbuild/src/rust-buildbot/slave/auto-mac-32-opt/build/src/librustc_driver/test.rs:141:11: 141:64 error: this function takes 2 parameters but 1 parameter was supplied [E0061]
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-32-opt/build/src/librustc_driver/test.rs:141     infcx.resolve_regions_and_report_errors(ast::CRATE_NODE_ID);
                                                                                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
make: *** [i686-apple-darwin/stage2/test/rustc_drivertest-i686-apple-darwin] Error 101

@nikomatsakis
Copy link
Contributor Author

@bors r=pnkfelix 55ffd2e

argh, had this locally but forgot to commit/push :(

@nikomatsakis
Copy link
Contributor Author

@bors p=1

@bors
Copy link
Contributor

bors commented Apr 24, 2015

⌛ Testing commit 55ffd2e with merge f9e53c7...

bors added a commit that referenced this pull request Apr 24, 2015
… r=pnkfelix

Rather than storing the relations between free-regions in a global
table, introduce a `FreeRegionMap` data structure. regionck computes the
`FreeRegionMap` for each fn and stores the result into the tcx so that
borrowck can use it (this could perhaps be refactored to have borrowck
recompute the map, but it's a bid tedious to recompute due to the
interaction of closures and free fns). The main reason to do this is
because of #22779 -- using a global table was incorrect because when
validating impl method signatures, we want to use the free region
relationships from the *trait*, not the impl.

Fixes #22779.
@bors bors merged commit 55ffd2e into rust-lang:master Apr 24, 2015
pnkfelix pushed a commit to pnkfelix/rust that referenced this pull request Apr 29, 2015
table, introduce a `FreeRegionMap` data structure. regionck computes the
`FreeRegionMap` for each fn and stores the result into the tcx so that
borrowck can use it (this could perhaps be refactored to have borrowck
recompute the map, but it's a bid tedious to recompute due to the
interaction of closures and free fns). The main reason to do this is
because of rust-lang#22779 -- using a global table was incorrect because when
validating impl method signatures, we want to use the free region
relationships from the *trait*, not the impl.

Fixes rust-lang#22779.

----

This is cherry-pick of commit 6dfeda7 from PR rust-lang#24553

Manually resolved conflicts in:
  src/librustc/lib.rs
  src/librustc/middle/infer/region_inference/mod.rs
(both conflicts were related to changes to file structure.)
pnkfelix pushed a commit to pnkfelix/rust that referenced this pull request Apr 29, 2015
This is a cherry-pick of commit 55ffd2e from PR rust-lang#24553
This was referenced Apr 29, 2015
pnkfelix pushed a commit to pnkfelix/rust that referenced this pull request Apr 29, 2015
This is cherry-pick of commit 55ffd2e from PR rust-lang#24553
alexcrichton added a commit that referenced this pull request Apr 29, 2015
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 29, 2015
@nikomatsakis nikomatsakis deleted the issue-22779-overconstrained-impl branch March 30, 2016 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Borrow checker allows slice lifetime to be extended
7 participants