Shouldn't lifetime elision apply to closure return type annotation?

[pnkfelix]

(Spawned off of #55526.)

Consider the following code (play):

fn foo(input: &str) -> &str {
    let id = |x: &str| -> &str { x };
    id(input)
}

The above does not compile today.

Error diagnostic:

error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
 --> src/main.rs:2:27
  |
2 |     let id = |x: &str| -> &str { x };
  |                           ^
  |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the body at 2:14...
 --> src/main.rs:2:14
  |
2 |     let id = |x: &str| -> &str { x };
  |              ^^^^^^^^^^^^^^^^^^^^^^^
note: ...so that reference does not outlive borrowed content
 --> src/main.rs:2:34
  |
2 |     let id = |x: &str| -> &str { x };
  |                                  ^
note: but, the lifetime must be valid for the anonymous lifetime #1 defined on the function body at 1:1...
 --> src/main.rs:1:1
  |
1 | / fn foo(input: &str) -> &str {
2 | |     let id = |x: &str| -> &str { x };
3 | |     id(input)
4 | | }
  | |_^
note: ...so that reference does not outlive borrowed content
 --> src/main.rs:3:5
  |
3 |     id(input)
  |     ^^^^^^^^^

However, discussions with @nikomatsakis lead us to wonder why lifetime elision is not being applied to the type annotation attached to the closure. Since there is a single input argument type and a single return type, it seems reasonable to apply lifetime elision, yielding the type for <'r> (&'r str) -> &'r str.

I believe fixing this is actually pretty trivial, based on some initial experimentation with the code here:

rust/src/librustc/middle/resolve_lifetime.rs

Lines 2112 to 2117 in 21cb46a

	// Everything else (only closures?) doesn't
	// actually enjoy elision in return types.
	_ => {
	self.visit_ty(output);
	return;
	}

(The question will only be whether there are unintended consequences of the fix; I'm still looking into that.)

(This might require an RFC, not sure yet.)

[pnkfelix]

Potentially related bugs (at least in terms of comparatively evaluating our specified guarantees and/or our currently implemented behavior):

• lifetime elision should apply to closure types #18637 -- at one time, generics like <F: Fn(&T, &T)> did not work, because it wanted explicit lifetimes in those reference types. Higher-ranked trait bounds rfcs#387 and PR Implement higher-ranked trait bounds #18639 addressed this, and it works today. (I mention it mainly because it sort of acts as evidence that the same elision rules should apply to type annotations on closure expressions, as discussed here.)
• Can’t declare lifetime for closure that returns a reference #22340 -- first two sentences of issue are: "When you declare closure argument types, there is no syntax to declare a lifetime parameter. And I guess lifetime elision does not apply to closures." Of course this issue (Shouldn't lifetime elision apply to closure return type annotation? #56537) proposes changing things so lifetime elisions can apply to closures (at least when they provide type annotations). But it would be good to extract corresponding test cases from the comments on that issue.
• Closure parameter inference and lifetime elision mayhem #24421 -- this discussed some bugs with mixing closures that have some explicit type annotations with contexts that have lifetime elision of the form `<F: Fn(&T, &T)>. But it didn't do anything with return types.
• Incorrect lifetime error with a generic FnMut after annotating the type in a closure. #22557 -- this discusses some issues with how regions in return types of closures are inferred based on what their surrounding context provides in terms of type information. But the closures in question always completely infer their return type; they never provide -> &_ of any sort; this puts them into a category that looks more like NLL error on closure, but not on equivalent function #55526 rather than this bug. Having said that, the comments in that ticket provide a lot of useful information about how the existing system ends up assigning types in potentially surprising ways.
• Lifetime elision is too greedy without explicit type declaration #39943 -- similar to Incorrect lifetime error with a generic FnMut after annotating the type in a closure. #22557, this discusses cases where lifetime elision was not behaving in the expected manner when one compares cases involving elision on the Fn trait bounds, e.g. F: Fn(&str) -> &str. And again like Incorrect lifetime error with a generic FnMut after annotating the type in a closure. #22557, the comment thread contains lots of useful information. But again, none of the examples involve an explicit return type on the closure expression; the only time the character sequence | -> & occurs is for hypothesized (but currently non-existant) syntax foo(for<'a> |&'a u32, &'a u32| -> &'a u32.

[pnkfelix]

Ugh. Here is some code, adapted from #23340, that my "fix" to this is going to "break":

fn print_first(list: Vec<String>) {
    let x: &str = list
        .first()
        .map(|s| -> &str { &s[..] })
        .unwrap_or("");
    println!("First element is {}", x);
}

That is, without the change described on this issue, the compiler accepts the above code.

But if I add the change so that lifetime elision is applied to the |s| -> &str, at least in a naive fashion, we get this error:

error[E0106]: missing lifetime specifier
 --> issue-22340-a.rs:6:21
  |
6 |         .map(|s| -> &str { &s[..] }) // works
  |                     ^ help: consider giving it an explicit bounded or 'static lifetime: `&'static`
  |
  = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments

[pnkfelix]

But if I add the change so that lifetime elision is applied to the |s| -> &str, at least in a naive fashion, we get this error:

the reason I wrote "at least in a naive fashion" is that the correct generalization of the lifetime elision rules to cover closure expressions is not trivially obvious. Why? Because those rules rely on definitions of lifetime positions in input/output, but the definitions given do not account for occurrences of the wildcard type _ (because that cannot occur in fn items nor impl headers).

And I might argue that it is the implicit occurrence of the wildcard type _ within |s| -> &str { ... } that is causing the case above to break when I added lifetime elision rules for |...| -> &T { ... }: because we do not (and cannot) syntactic identify any input lifetimes with the wildcard, it causes us to treat this as the case where there are no lifetime positions, and thus it is an error to elide the output lifetime on &str.

[pnkfelix]

But one way to generalize the rules in a manner that maintains backward compatibility might be to say: If there are any wildcard types _ on the input types of a closure, then we make no attempt to apply lifetime elision to the return type (and instead fallback on region inference).

This may yield surprising differences in behavior for end-users depending on which types annotations are supplied and which are not. But I think that is inevitable when you get into the weeds like this.

[nikomatsakis]

I left some thoughts on Zulip:

The TL;DR is that "annotations on closures are tricky". It feels like lifetime elision should apply (and I argued this before) -- but it's clear that can break code in light of how we handle the expected/provided split.

Right now, if there is an expected signature ES and a provided signature PS, we basically check that there exists some substitution Θ such that Θ[PS] = ES. This means that e.g. |x: &str, y: &str| would match an expected signature of for<'a> fn(&'a str, &'a str), even though the elision rules for a top-level fn would expand to for<'a, 'b> fn(&'a str, &'b str). Similar logic applies to elision.

I could imagine trying to "defer" the elision rules until we know if there is an expected type...but ugh.

[pnkfelix]

yet another option would be to sidestep trying to resolve this via elision, and instead focus attention on adding support for an expression form for <'r> CLOSURE so that we could do for <'r> |s: &'r _| -> &'r _ { ... }. (Which I think is issue #22340)

[pnkfelix]

By the way...

I wrote:

But if I add the change so that lifetime elision is applied to the |s| -> &str, at least in a naive fashion, we get this error: ...

and @nikomatsakis subsequently referenced some comments on Zulip, which included this:

applying elision makes this break in some cases
e.g., |&str| -> &str

To make this quite concrete, I made the following example (play)

fn willy<'w>(p: &'w str) -> &'w str {
    let free_dumb = |x: &str| -> &str { p };
    let hello = format!("Hello");
    free_dumb(&hello)
}

fn main() {
    let world = format!("World");
    willy(&world);
}

In the above, the closure assigned to free_dumb has the signature |x: &str| -> &str, but it is relying on elision not applying. It doesn't want the type for <'r> fn(&'r str) -> &'r str; it wants the type for <'r> fn(&'r str) -> &'w str, where 'w is the lifetime on the input parameter.

And indeed, the compiler today accepts that code as written.

While my naive change to the compiler rejects it, with this error message from AST-borrowck:

error[E0312]: lifetime of reference outlives lifetime of borrowed content...
 --> free-willy.rs:2:41
  |
2 |     let free_dumb = |x: &str| -> &str { p };
  |                                         ^
  |
note: ...the reference is valid for the anonymous lifetime #1 defined on the body at 2:21...
 --> free-willy.rs:2:21
  |
2 |     let free_dumb = |x: &str| -> &str { p };
  |                     ^^^^^^^^^^^^^^^^^^^^^^^
note: ...but the borrowed content is only valid for the lifetime 'w as defined on the function body at 1:10
 --> free-willy.rs:1:10
  |
1 | fn willy<'w>(p: &'w str) -> &'w str {
  |          ^^

and this error message from NLL borrowck:

error: unsatisfied lifetime constraints
 --> free-willy.rs:2:41
  |
1 | fn willy<'w>(p: &'w str) -> &'w str {
  |          -- lifetime `'w` defined here
2 |     let free_dumb = |x: &str| -> &str { p };
  |                                         ^ returning this value requires that `'w` must outlive `'static`

error[E0597]: `hello` does not live long enough
 --> free-willy.rs:4:15
  |
2 |     let free_dumb = |x: &str| -> &str { p };
  |                                         - returning this value requires that `hello` is borrowed for `'static`
3 |     let hello = format!("Hello");
4 |     free_dumb(&hello)
  |               ^^^^^^ borrowed value does not live long enough
5 | }
  | - `hello` dropped here while still borrowed

[pnkfelix]

if nothing else, I would like to encode the examples from the comments on this issue into tests that we put into our test suite. Because right now, if I add the naive change to apply lifetime elision rules to |s| -> &str (and |s: &str| -> &str, etc), our test suite passes, because it does not cover the tests that have been pointed out here.