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

ignore higher-ranked object bound conditions created by WF #59132

Merged
merged 2 commits into from Mar 13, 2019

Conversation

Projects
None yet
3 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 12, 2019

In the issue-53548 test added in this PR, the Box<dyn Trait> type is expanded to Box<dyn Trait + 'static>, but the generator "witness" that results is for<'r> { Box<dyn Trait + 'r> }. The WF code was encountering an ICE (when debug-assertions were enabled) and an unexpected compilation error (without debug-asserions) when trying to process this 'r region bound. In particular, to be WF, the region bound must meet the requirements of the trait, and hence we got for<'r> { 'r: 'static }. This would ICE because the Binder constructor we were using was assering that no higher-ranked regions were involved (because the WF code is supposed to skip those). The error (if debug-asserions were disabled) came because we obviously cannot prove that 'r: 'static for any region 'r. Pursuant with
our "lazy WF" strategy for higher-ranked regions, the fix is not to require that for<'r> { 'r: 'static } holds (this is also analogous to what we would do for higher-ranked regions appearing within the trait in other positions).

Fixes #53548

r? @pnkfelix

nikomatsakis added some commits Mar 12, 2019

ignore higher-ranked WF requirements for trait objects
In the `issue-53548` test added in this commit, the `Box<dyn Trait>`
type is expanded to `Box<dyn Trait + 'static>`, but the generator
"witness" that results is `for<'r> { Box<dyn Trait + 'r> }`. The WF
code was encountering an ICE (when debug-assertions were enabled) and
an unexpected compilation error (without debug-asserions) when trying
to process this `'r` region bound. In particular, to be WF, the region
bound must meet the requirements of the trait, and hence we got
`for<'r> { 'r: 'static }`. This would ICE because the `Binder`
constructor we were using was assering that no higher-ranked regions
were involved (because the WF code is supposed to skip those). The
error (if debug-asserions were disabled) came because we obviously
cannot prove that `'r: 'static` for any region `'r`.  Pursuant with
our "lazy WF" strategy for higher-ranked regions, the fix is not to
require that `for<'r> { 'r: 'static }` holds (this is also analogous
to what we would do for higher-ranked regions appearing within the
trait in other positions).

@pnkfelix pnkfelix self-assigned this Mar 12, 2019

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 12, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 12, 2019

📌 Commit 261daf2 has been approved by pnkfelix

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 12, 2019

@bors rollup

Centril added a commit to Centril/rust that referenced this pull request Mar 13, 2019

Rollup merge of rust-lang#59132 - nikomatsakis:issue-53548-generator-…
…bound, r=pnkfelix

ignore higher-ranked object bound conditions created by WF

In the `issue-53548` test added in this PR, the `Box<dyn Trait>` type is expanded to `Box<dyn Trait + 'static>`, but the generator "witness" that results is `for<'r> { Box<dyn Trait + 'r> }`. The WF code was encountering an ICE (when debug-assertions were enabled) and an unexpected compilation error (without debug-asserions) when trying to process this `'r` region bound. In particular, to be WF, the region bound must meet the requirements of the trait, and hence we got `for<'r> { 'r: 'static }`. This would ICE because the `Binder` constructor we were using was assering that no higher-ranked regions were involved (because the WF code is supposed to skip those). The error (if debug-asserions were disabled) came because we obviously cannot prove that `'r: 'static` for any region `'r`.  Pursuant with
our "lazy WF" strategy for higher-ranked regions, the fix is not to require that `for<'r> { 'r: 'static }` holds (this is also analogous to what we would do for higher-ranked regions appearing within the trait in other positions).

Fixes rust-lang#53548

r? @pnkfelix

bors added a commit that referenced this pull request Mar 13, 2019

Auto merge of #59151 - Centril:rollup, r=Centril
Rollup of 16 pull requests

Successful merges:

 - #58829 (librustc_interface: Update scoped-tls to 1.0)
 - #58876 (Parse lifetimes that start with a number and give specific error)
 - #58908 (Update rand version)
 - #58998 (Fix documentation of from_ne_bytes and from_le_bytes)
 - #59056 (Use lifetime contravariance to elide more lifetimes in core+alloc+std)
 - #59057 (Standardize `Range*` documentation)
 - #59080 (Fix incorrect links in librustc_codegen_llvm documentation)
 - #59083 (Fix #54822 and associated faulty tests)
 - #59093 (Remove precompute_in_scope_traits_hashes)
 - #59101 (Reduces Code Repetitions like `!n >> amt`)
 - #59121 (impl FromIterator for Result: Use assert_eq! instead of assert!)
 - #59124 (Replace assert with assert_eq)
 - #59129 (Visit impl Trait for dead_code lint)
 - #59130 (Note that NonNull does not launder shared references for mutation)
 - #59132 (ignore higher-ranked object bound conditions created by WF)
 - #59138 (Simplify Iterator::{min, max})

Failed merges:

r? @ghost

@bors bors merged commit 261daf2 into rust-lang:master Mar 13, 2019

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.