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

In-band lifetimes stop working when in a nested function #52532

Closed
Tracked by #44524
Mark-Simulacrum opened this issue Jul 19, 2018 · 8 comments · Fixed by #63501
Closed
Tracked by #44524

In-band lifetimes stop working when in a nested function #52532

Mark-Simulacrum opened this issue Jul 19, 2018 · 8 comments · Fixed by #63501
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

#![feature(rust_2018_preview)]

fn main() {}

struct Bar<'a>(&'a u32);

impl Bar<'a> {
    fn foo() {
        fn foo(_: &'a u32) { } // error: use of undeclared lifetime name `'a`
    }
}

https://play.rust-lang.org/?gist=d2b32bc0947a05fa0830a0d244dcb4fa&version=nightly&mode=debug&edition=2015

@petrochenkov
Copy link
Contributor

The diagnostics ("use of undeclared lifetime name") are bad, but this indeed must be an error because the inner 'a is an illegal use of the outer 'a rather than an implicitly defined fresh lifetime.

@Mark-Simulacrum Mark-Simulacrum added this to the Rust 2018 Preview 2 milestone Jul 19, 2018
@Mark-Simulacrum Mark-Simulacrum added the A-rust-2018-preview Area: The 2018 edition preview label Jul 19, 2018
@Mark-Simulacrum
Copy link
Member Author

Mark-Simulacrum commented Jul 19, 2018

Hm, I don't think that's quite what I'd expect: previously using <'a> in that context would be fine because there is no shadowing so to speak, that is, this compiles:

fn main() {}

struct Bar<'a>(&'a u32);

impl<'a> Bar<'a> {
    fn foo() {
        fn foo<'a>(_: &'a u32) { }
    }
}

@petrochenkov
Copy link
Contributor

From https://github.com/aturon/rfcs/blob/93259c2fb9a66d805d9f5c9c049df82aad2b5553/text/2115-argument-lifetimes.md#lifetimes-in-fn-signatures

Similarly, if a fn definition is nested inside another fn definition, it is an error to mention lifetimes from that outer definition (without binding them explicitly). This is again intended for future-proofing and clarity, and is an edge case.

(In the second example there's an explicit definition that shadows the previous definition for following uses, in the first example there's no second definition, only use)

@Mark-Simulacrum
Copy link
Member Author

I see that this is in the RFC, but I think I more or less disagree with the reasoning given. Disallowing the use of inband lifetimes when nesting function seems arbitrary and not entirely helpful; it also makes a mechanical change from "old" to "new" more difficult because shadowing is visible only based on indent levels.

@durka
Copy link
Contributor

durka commented Jul 19, 2018

IMO it should be an error, with a helpful note like the one you get when writing fn foo<T>() { fn foo(_: T) {} }. Let's not make in-band lifetimes any more confusing than they already are.

@Mark-Simulacrum
Copy link
Member Author

The distinction is that inband lifetime's selling point is that you don't need to declare lifetimes. If we say that, and then clarify it with "except" then I think the feature feels incomplete. Inband lifetimes confusion does not increase with permitting this, IMO.

@durka
Copy link
Contributor

durka commented Jul 19, 2018

The problem is if it's allowed, then

impl Foo<'a> { fn foo(_: &'a u32) }

has a different meaning than

fn foo(_: &'a u32) { fn foo(_: &'a u32) {} }

This seems unfortunate and confusing. Even unergonomic if I may say so. There's no way that the distinction between 'a and 'a won't be missed in a quick scan of the second one. But, like, the RFC process did decide that isn't important, so whatever.

@Mark-Simulacrum
Copy link
Member Author

Well, somewhat, but not really. Nothing outside of imports/definitions inherits into the inner function, unlike the impl where there is a history and multiple things which inherit (Self, lifetimes, generics).

@Mark-Simulacrum Mark-Simulacrum added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 24, 2018
@aturon aturon removed this from the Rust 2018 RC milestone Sep 5, 2018
@scottmcm scottmcm removed the A-rust-2018-preview Area: The 2018 edition preview label Dec 3, 2018
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
…-impl-lifetime, r=cramertj

use `ParamName` to track in-scope lifetimes instead of Ident

Also, clear in-scope lifetimes when visiting nested items.

Fixes rust-lang#63500.
Fixes rust-lang#63225.
Fixes rust-lang#52532.

r? @cramertj
Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019
…-impl-lifetime, r=cramertj

use `ParamName` to track in-scope lifetimes instead of Ident

Also, clear in-scope lifetimes when visiting nested items.

Fixes rust-lang#63500.
Fixes rust-lang#63225.
Fixes rust-lang#52532.

r? @cramertj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants