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

Underscore lifetimes are incorrectly accepted as lifetime bounds in impl headers #54902

Closed
scottmcm opened this issue Oct 8, 2018 · 8 comments · Fixed by #55162
Closed

Underscore lifetimes are incorrectly accepted as lifetime bounds in impl headers #54902

scottmcm opened this issue Oct 8, 2018 · 8 comments · Fixed by #55162
Assignees
Labels
C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Oct 8, 2018

This is currently accepted in 2018: https://play.rust-lang.org/?gist=7ba2dee1134f9848b54b7364ea9568aa&version=nightly&edition=2018

trait Foo<'a> {}
impl<'b: '_> Foo<'b> for i32 {}

AFAICT, it desugars to

impl<'b: 'c, 'c> Foo<'b> for i32 {}

So it's "fine", but useless.

It should error, as trying to do this does in other places, such as

struct Foo<'a: '_>(&'a i32);

Tracking issue for the impl_header_lifetime_elision feature: #15872

On the current beta this gives

error[E0688]: cannot mix in-band and explicit lifetime definitions

so this was at least exposed by #54458

@scottmcm scottmcm changed the title Underscore lifetimes are accepted as lifetime bounds in impl headers Underscore lifetimes are incorrectly accepted as lifetime bounds in impl headers Oct 8, 2018
@Centril Centril added P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Oct 8, 2018
@Centril Centril added this to the Edition 2018 RC 2 milestone Oct 8, 2018
@nikomatsakis nikomatsakis self-assigned this Oct 11, 2018
@pnkfelix
Copy link
Member

visited for triage. Agreed that P-high and RC2 milestones are appropriate. @nikomatsakis is going to work first on mentorship; if that fails to find a volunter, they will fallback to fixing it themself.

@nikomatsakis
Copy link
Contributor

Actually, I'm not sure if this is an error or not. We currently permit '_ in other, similar contexts.

Example 1 (playground):

trait Foo<'a> { }

struct Bar<T> { t: T }

impl<T: Foo<'_>> Bar<T> { }

fn main() { }

and example 2 (playground):

trait Foo<'a> { }

struct Bar<T> { t: T }

impl<T> Bar<T> where T: Foo<'_> { }

fn main() { }

Are we comfortable with those, but just not this? (I guess because this is clearly useless, whereas those .. may be useful?)

@nikomatsakis
Copy link
Contributor

For some reason, I thought maybe we were not going to permit '_ in those contexts, out of fear that the semantics were not what we might want. In particular, I can imagine that people might expect T: Foo<'_> to be equivalent to T: for<'a> Foo<'a>.

@nikomatsakis nikomatsakis added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 11, 2018
@nikomatsakis
Copy link
Contributor

Survey of current behavior.

In Rust 2018 on nightly, all of the following are accepted.

Using '_ in a where clause (inherent impl)

Playground.

trait WithType<T> {}
trait WithRegion<'a> { }

struct Foo<T> { 
    t: T
}

impl<T> Foo<T>
where T: WithRegion<'_> { }

fn main() {}

Using &u32 in a where clause (inherent impl)

Playground.

trait WithType<T> {}
trait WithRegion<'a> { }

struct Foo<T> { 
    t: T
}

impl<T> Foo<T>
where T: WithType<&u32> { }

fn main() {}

Using &u32 in a where clause (trait impl)

Playground.

trait WithType<T> {}
trait WithRegion<'a> { }

trait Foo { }

impl<T> Foo for Vec<T>
where T: WithType<&u32> { }

fn main() {}

Using '_ in a where clause (trait impl)

Playground.

trait WithType<T> {}
trait WithRegion<'a> { }

trait Foo { }

impl<T> Foo for Vec<T>
where T: WithRegion<'_> { }

fn main() {}

@nikomatsakis
Copy link
Contributor

Survey of current behavior.

In Rust 2015 on nightly, all are errors.

@nikomatsakis
Copy link
Contributor

We discussed this in the @rust-lang/lang meeting and there seemed to be consensus that we ought to make it an error to use '_ or &u32 in a where-clause for the time being, so as to leave room for locating the "implicit binder" at a different spot in the future. I think there is a connection here between this and in-band lifetimes, since they must also specify an "implicit binder", and hence it probably makes sense to "wait and see" where they go first.

(I have first hand observed confusion on this point.)

Given that the behavior is only accepted in Rust 2018, it seems we still have room to change this without requiring backporting.

@nikomatsakis
Copy link
Contributor

See also #45667

@nikomatsakis
Copy link
Contributor

Fixed in #55162

bors added a commit that referenced this issue Oct 19, 2018
…mandry

handle underscore bounds in unexpected places

Per the discussion on #54902, I made it a hard error to use lifetime bounds in various places where they used to be permitted:

- `where Foo: Bar<'_>` for example

I also moved error reporting to HIR lowering and added `Error` variants to let us suppress downstream errors that result.

I (imo) improved the error message wording to be clearer, as well.

In the process, I fixed the ICE in #52098.

Fixes #54902
Fixes #52098
bors added a commit that referenced this issue Oct 23, 2018
Stabilize impl_header_lifetime_elision in 2015

~~This is currently blocked on #54902; it should be good after that~~

It's already stable in 2018; this finishes the stabilization.

FCP completed (#15872 (comment)), proposal (#15872 (comment)).

Tracking issue: #15872
Usage examples (from libcore): #54687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants