Passing a `Fn` to an `fn` now requires explicit lifetimes for the `Fn` and all references passed to it when invoked

[jmesmon]

use std::env;
use std::ffi::OsString;

/* Explicit lifetimes are required by rust-2015-05-11 nightly (newer than playpen at the moment)
 * Previously implicit lifetimes worked fine */
fn foo<'a, F>(bar: &'a F) -> Option<OsString>
    where F: Fn(&'a str) -> Option<OsString>
{
    bar("HOME")
}

fn main() {
    println!("{:?}", foo(&env::var_os));
}

Playpen: http://is.gd/LYhXZx

Might be caused by: #25212

I'm unsure if this is an actual issue that can be fixed, but figured I'd open an issue in case it was.

[mbrubeck]

Or this might be an intentional breakage from #24461.

[pnkfelix]

I don't see Box in the example, so I am skeptical that this would be #25212 ...

[bluss]

This is a regression from Rust 1.0

[huonw]

cc @nikomatsakis

[pnkfelix]

There is also this approach to working around the problem: use a for <'a> ... in the trait bound to bind the lifetime. (But doing this apparently requires eta-expanding the fn, wrapping it with a trivial closure; I'm not clear about why that is necessary...)

use std::env;
use std::ffi::OsString;

fn foo<F>(bar: &F) -> Option<OsString>
    where F: for <'r> Fn(&'r str) -> Option<OsString>
{
    bar("HOME")
}

fn main() {
    println!("{:?}", foo(&|x|env::var_os(x)));
}

[jmesmon]

Playpen of the failing-on-nightly-but-not-stable version, for reference:

http://is.gd/UIC7SX (playpen is now on a nightly new enough to reproduce the failure, despite the inline comment)

[brson]

Nominating because @bluss suggested this was a stable regression.

[jmesmon]

(Just noticed I hadn't included the error message)

<anon>:12:22: 12:25 error: type mismatch: the type `fn(_) -> core::option::Option<std::ffi::os_str::OsString> {std::env::var_os}` implements the trait `core::ops::Fn<(_,)>`, but the trait `for<'r> core::ops::Fn<(&'r str,)>` is required (expected concrete lifetime, found bound lifetime parameter ) [E0281]
<anon>:12     println!("{:?}", foo(&env::var_os));
                               ^~~

[nikomatsakis]

Hmm. I do think that this should work. If you edit the example to use a crate-local fn, for example, it does work: http://is.gd/vaEi9C

use std::env;
use std::ffi::{OsStr, OsString};

fn test<K:?Sized>(_: &K) -> Option<OsString> where K: AsRef<OsStr> { None }

/* Explicit lifetimes are required by rust-2015-05-11 nightly (newer than playpen) */
fn foo<'a, F>(bar: &'a F) -> Option<OsString>
    where F: Fn(&str) -> Option<OsString>
{
    bar("HOME")
}

fn main() {
    println!("{:?}", foo(&test));
}

[nikomatsakis]

triage: P-high

I'm going to mark this as P-high because I agree it is a regression. My guess would be that perhaps it is related to #24615 or some of the other recent changes to trait selection, but I'll have to dive in and trace it out.

[nikomatsakis]

So, I've tracked down the origin, but the code in question has landed in stable already. The cause was commit 9b88cd1. This modified the definition of var_os as follows:

@@ -197,7 +197,7 @@ pub fn var<K: ?Sized>(key: &K) -> Result<String, VarError> where K: AsRef<OsStr>
 /// }
 /// ```
 #[stable(feature = "env", since = "1.0.0")]
-pub fn var_os<K: ?Sized>(key: &K) -> Option<OsString> where K: AsRef<OsStr> {
+pub fn var_os<K: AsRef<OsStr>>(key: K) -> Option<OsString> {
     let _g = ENV_LOCK.lock();
     os_imp::getenv(key.as_ref())
 }

One effect of this definition is that var_os no longer satisfies the type for<'r> fn(&'r K) but rather the type fn(&'r K). The difference is subtle -- the first type says that you can call this function pointer multiple times with distinct lifetimes each time, the second says that the function pointer is associated with a single lifetime for each reference. In other words, if you write multiple calls to var_os(), you'll never notice the difference, but if you capture a single reference to var_os, you might:

let x = var_os;
x(..);
x(..);

vs

var_os(..);
var_os(..);

In this case, the code was capturing a single reference to var_os and using it as a closure. Converting the code to use explicit lifetimes changed the closure so that it only accepts references with the lifetime 'a, versus any lifetime. Another fix would be to change from foo(&env::var_os) to foo(|x| env::var_os(x)).

I'm going to close this, since the code has made it to stable and doesn't seem to have caused dramatic impact, but it's an interesting regression vector.

cc @rust-lang/libs <-- this should be kept in mind in the future.

[nikomatsakis]

Ah, I see @pnkfelix suggested the second change already. The reason that this works is that it is equivalent to making one reference to var_os per call-site.