make lifetimes that only appear in return type early-bound #38897

nikomatsakis · 2017-01-06T23:55:23Z

This is the full and proper fix for #32330. This also makes some effort to give a nice error message (as evidenced by the ui test), sending users over to the tracking issue for a fuller explanation and offering a --explain message in some cases.

This needs a crater run before we land.

r? @arielb1

nikomatsakis · 2017-01-06T23:56:04Z

Oh I forgot-- I should deprecate the older fwds-compat lint too. And maybe improve wording of error message? Not sure how to do that.

nikomatsakis · 2017-01-09T19:56:04Z

nikomatsakis · 2017-01-09T20:04:27Z

Some of the failures above may be addressed by fixing #33684, though I'm not sure. In any case, I can try to submit PRs to address the issues.

arielb1 · 2017-01-19T15:57:28Z

Will review later today. Sorry I missed this.

arielb1 · 2017-01-19T16:47:23Z

src/librustc/middle/resolve_lifetime.rs

-    // are named regions appearing in fn arguments that do not appear
-    // in where-clauses
-    pub late_bound: NodeMap<ty::Issue32330>,
+    // the set of lifetime def ids that are early-bound; early-bound


Should be "that are late-bound".

arielb1 · 2017-01-19T16:49:21Z

src/librustc/ty/mod.rs

@@ -637,8 +638,7 @@ impl<'tcx> RegionParameterDef<'tcx> {
    }

    pub fn to_bound_region(&self) -> ty::BoundRegion {
-        // this is an early bound region, so unaffected by #32330
-        ty::BoundRegion::BrNamed(self.def_id, self.name, Issue32330::WontChange)
+        ty::BoundRegion::BrNamed(self.def_id, self.name, self.issue_32330)


So we're having that big enum on every ty::Region? I suppose we should intern the (DefId, Name, Issue32230) triple and check the perf impact. Maybe just do bitpacking to fit BoundRegion in a u32 - should check the perf impact of that.

arielb1

r+ after we communicate with the crater crate authors and fix nits.

arielb1 · 2017-01-20T21:51:00Z

tidy check (x86_64-unknown-linux-gnu)

duplicate error code: 573

/checkout/src/librustc_typeck/diagnostics.rs:4138:     E0573: r##"

/checkout/src/librustc_resolve/diagnostics.rs:1544:     E0573,

duplicate error code: 574

/checkout/src/librustc_typeck/diagnostics.rs:4175:     E0574: r##"

/checkout/src/librustc_resolve/diagnostics.rs:1545:     E0574,

thread 'main' panicked at 'some tidy checks failed', /checkout/src/tools/tidy/src/main.rs:55

note: Run with `RUST_BACKTRACE=1` for a backtrace.

nikomatsakis · 2017-01-24T22:01:13Z

@arielb1 ok I will follow-up prob tomorrow...

nikomatsakis · 2017-01-25T22:16:15Z

~~OK, this PR is ready to land~~. OK, this PR is almost ready to land, see below. =)

The following are the "core crates" that appear to be affected by this change:

lmdb-rs (cc @vhbit) -- fix incorrect &self annotation vhbit/lmdb-rs#42
doubly (cc @marchelzo) -- I've validated that this is fixed on doubly master
pippin (cc @dhardy) -- fix incorrect impl method declarations dhardy/pippin#53

These crates will have to be fixed -- hopefully they are getting a lint warning now about future compatibility, though I'm not sure if the lint covers all the cases that are affected by the proper fix (that is quite challenging). Let me know if you are having trouble updating -- if I find time, I will try to prep some PRs. =)

In addition, a number of crates were affected indirectly, because they relied on an older serde release that is affected. In these cases, the ideal thing is that they would update to a modern version of serde:

rusted-cypher (cc @livioribeiro)
nametable (cc @crlf0710)
pandoc-ast (cc @oli-obk)
parity-dapps-glue (cc @ethcore)
presto and presto-rs (cc @ernestrc, but these appear to be unavailable in GitHub)

We could also solve this by a new point release of the relevant serde crates (cc @dtolnay, @erickt).

UPDATE: @erickt helpfully patched serde, so this should be ok.

nikomatsakis · 2017-01-25T22:17:04Z

@arielb1

So we're having that big enum on every ty::Region? I suppose we should intern the (DefId, Name, Issue32230) triple and check the perf impact. Maybe just do bitpacking to fit BoundRegion in a u32 - should check the perf impact of that.

Ah, I missed this comment. Yes, that's a reasonable point. I'll take a look and see what I can do.

nikomatsakis · 2017-01-26T20:11:25Z

@arielb1

So we're having that big enum on every ty::Region? I suppose we should intern the (DefId, Name, Issue32230) triple and check the perf impact. Maybe just do bitpacking to fit BoundRegion in a u32 - should check the perf impact of that.

Fixed in latest commit. PR should be good to go.

eddyb · 2017-01-27T18:54:19Z

cycling Travis to force a build from it

bors · 2017-01-28T00:47:34Z

☔ The latest upstream changes (presumably #37057) made this pull request unmergeable. Please resolve the merge conflicts.

This is a problem with the code uncovered by the fix in rust-lang/rust#38897

This is the full and proper fix for rust-lang#32330. This also makes some effort to give a nice error message (as evidenced by the `ui` test), sending users over to the tracking issue for a full explanation.

These were found due to the compiler fixes in rust-lang/rust#38897

nikomatsakis · 2017-02-05T21:58:19Z

@bors r=arielb1

bors · 2017-02-05T21:58:20Z

📌 Commit b26120d has been approved by arielb1

bors · 2017-02-05T22:53:14Z

⌛ Testing commit b26120d with merge a3da24b...

@arielb1

make lifetimes that only appear in return type early-bound This is the full and proper fix for #32330. This also makes some effort to give a nice error message (as evidenced by the `ui` test), sending users over to the tracking issue for a fuller explanation and offering a `--explain` message in some cases. This needs a crater run before we land. r? @arielb1

bors · 2017-02-06T01:47:18Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing a3da24b to master...

nikomatsakis · 2017-03-02T20:31:45Z

Tagging this for release notes. Something like this:

"Lifetime parameters that do not appear in the arguments are now considered early-bound, resolving a soundness bug (#32330). The hr_lifetime_in_assoc_type future-compatibility lint has been in effect since April of 2016."

rust-highfive assigned arielb1 Jan 6, 2017

arielb1 reviewed Jan 19, 2017

View reviewed changes

arielb1 suggested changes Jan 19, 2017

View reviewed changes

nikomatsakis force-pushed the issue-32330-followup branch 2 times, most recently from 2fcd11c to eaa72d7 Compare January 25, 2017 22:11

erickt mentioned this pull request Jan 25, 2017

Get 0.29.x to compile on latest nightly serde-deprecated/syntex#113

Merged

nikomatsakis force-pushed the issue-32330-followup branch from 0c03295 to c803efb Compare January 26, 2017 21:35

eddyb closed this Jan 27, 2017

eddyb reopened this Jan 27, 2017

nikomatsakis force-pushed the issue-32330-followup branch 3 times, most recently from e8e60a6 to a611487 Compare February 5, 2017 20:56

nikomatsakis added a commit to nikomatsakis/lmdb-rs that referenced this pull request Feb 5, 2017

fix incorrect &self annotation

4fa8cea

This is a problem with the code uncovered by the fix in rust-lang/rust#38897

nikomatsakis mentioned this pull request Feb 5, 2017

fix incorrect &self annotation vhbit/lmdb-rs#42

Merged

make lifetimes that only appear in return type early-bound

b26120d

This is the full and proper fix for rust-lang#32330. This also makes some effort to give a nice error message (as evidenced by the `ui` test), sending users over to the tracking issue for a full explanation.

nikomatsakis force-pushed the issue-32330-followup branch from a611487 to b26120d Compare February 5, 2017 21:31

nikomatsakis added a commit to nikomatsakis/pippin that referenced this pull request Feb 5, 2017

fix incorrect impl method declarations

413525e

These were found due to the compiler fixes in rust-lang/rust#38897

nikomatsakis mentioned this pull request Feb 5, 2017

fix incorrect impl method declarations dhardy/pippin#53

Merged

bors merged commit b26120d into rust-lang:master Feb 6, 2017

nikomatsakis mentioned this pull request Mar 2, 2017

Unsound projection when late-bound-region appears only in return type #32330

Closed

nikomatsakis added the relnotes Marks issues that should be documented in the release notes of the next release. label Mar 2, 2017

This was referenced Apr 2, 2017

hr_lifetime_in_assoc_type future-compatibility warnings #33685

Closed

Regression in nametable_codegen-0.1.2, Rust 1.17 #40960

Closed

Regression in botocore_parser-0.1.1, Rust 1.17 #40954

Closed

nikomatsakis deleted the issue-32330-followup branch April 14, 2017 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make lifetimes that only appear in return type early-bound #38897

make lifetimes that only appear in return type early-bound #38897

nikomatsakis commented Jan 6, 2017 •

edited

Loading

nikomatsakis commented Jan 6, 2017

nikomatsakis commented Jan 9, 2017 •

edited

Loading

nikomatsakis commented Jan 9, 2017

arielb1 commented Jan 19, 2017

arielb1 Jan 19, 2017

arielb1 Jan 19, 2017 •

edited

Loading

arielb1 left a comment

arielb1 commented Jan 20, 2017

nikomatsakis commented Jan 24, 2017

nikomatsakis commented Jan 25, 2017 •

edited

Loading

nikomatsakis commented Jan 25, 2017

nikomatsakis commented Jan 26, 2017

eddyb commented Jan 27, 2017

bors commented Jan 28, 2017

nikomatsakis commented Feb 5, 2017

bors commented Feb 5, 2017

bors commented Feb 5, 2017

bors commented Feb 6, 2017

nikomatsakis commented Mar 2, 2017 •

edited

Loading

make lifetimes that only appear in return type early-bound #38897

make lifetimes that only appear in return type early-bound #38897

Conversation

nikomatsakis commented Jan 6, 2017 • edited Loading

nikomatsakis commented Jan 6, 2017

nikomatsakis commented Jan 9, 2017 • edited Loading

Root regressions, sorted by rank:

nikomatsakis commented Jan 9, 2017

arielb1 commented Jan 19, 2017

arielb1 Jan 19, 2017

Choose a reason for hiding this comment

arielb1 Jan 19, 2017 • edited Loading

Choose a reason for hiding this comment

arielb1 left a comment

Choose a reason for hiding this comment

arielb1 commented Jan 20, 2017

nikomatsakis commented Jan 24, 2017

nikomatsakis commented Jan 25, 2017 • edited Loading

nikomatsakis commented Jan 25, 2017

nikomatsakis commented Jan 26, 2017

eddyb commented Jan 27, 2017

bors commented Jan 28, 2017

nikomatsakis commented Feb 5, 2017

bors commented Feb 5, 2017

bors commented Feb 5, 2017

bors commented Feb 6, 2017

nikomatsakis commented Mar 2, 2017 • edited Loading

nikomatsakis commented Jan 6, 2017 •

edited

Loading

nikomatsakis commented Jan 9, 2017 •

edited

Loading

arielb1 Jan 19, 2017 •

edited

Loading

nikomatsakis commented Jan 25, 2017 •

edited

Loading

nikomatsakis commented Mar 2, 2017 •

edited

Loading