Regression: "temporary value does not live long enough" in a static item in Servo

[SimonSapin]

Reduced test case:

#![feature(const_fn)]
const fn require_static<T>(_: &'static T) {}
const fn foo() -> u32 { 0 }
pub static A: () = require_static(&foo());

rustc 1.31.0-nightly (5597ee8 2018-10-03): OK

rustc 1.31.0-nightly (8c4ad4e 2018-10-04):

error[E0597]: borrowed value does not live long enough
 --> a.rs:4:36
  |
4 | pub static A: () = require_static(&foo());
  |                                    ^^^^^- temporary value only lives until here
  |                                    |
  |                                    temporary value does not live long enough
  |
  = note: borrowed value must be valid for the static lifetime...

error: aborting due to previous error

Commit range: 5597ee8...8c4ad4e. #53851 seems relevant. @oli-obk, should we use #[rustc_promotable] in unstable code, despite the rustc_ prefix in its name?

[SimonSapin]

Hmm, this diff does not seem to have any effect on the 275 occurrences of this error in Servo, am I missing something:

diff --git a/components/script/dom/bindings/interface.rs b/components/script/dom/bindings/interface.rs
index 210e9ae557..7632d341b1 100644
--- a/components/script/dom/bindings/interface.rs
+++ b/components/script/dom/bindings/interface.rs
@@ -88,6 +88,7 @@ pub struct InterfaceConstructorBehavior(ClassOps);
 
 impl InterfaceConstructorBehavior {
     /// An interface constructor that unconditionally throws a type error.
+    #[rustc_promotable]
     pub const fn throw() -> Self {
         InterfaceConstructorBehavior(ClassOps {
             addProperty: None,
@@ -105,6 +106,7 @@ impl InterfaceConstructorBehavior {
     }
 
     /// An interface constructor that calls a native Rust function.
+    #[rustc_promotable]
     pub const fn call(hook: ConstructorClassHook) -> Self {
         InterfaceConstructorBehavior(ClassOps {
             addProperty: None,
diff --git a/components/script/lib.rs b/components/script/lib.rs
index 097f69d780..a5355a3937 100644
--- a/components/script/lib.rs
+++ b/components/script/lib.rs
@@ -7,6 +7,7 @@
 #![feature(const_fn)]
 #![feature(drain_filter)]
 #![feature(plugin)]
+#![feature(rustc_attrs)]
 #![feature(try_from)]
 #![deny(unsafe_code)]
 #![allow(non_snake_case)]

Sample error in non-reduced code:

error[E0597]: borrowed value does not live long enough
    --> /home/simon/servo2/target/debug/build/script-1105d98fbcf10485/out/Bindings/AnalyserNodeBinding.rs:1522:10
     |
1522 |         &InterfaceConstructorBehavior::call(_constructor),
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
...
1525 |         2);
     |          - temporary value only lives until here
     |
     = note: borrowed value must be valid for the static lifetime...

[oli-obk]

hmm... I thought const_fn was overriding any rules. And... I just realized that's only true for nll ;) So add #![feature(nll)]

Or you can always use an intermediate constant.

#[rustc_promotable] is expected to stay unstable forever, but you can use it if you want. You're on your own with guarantees though.

[oli-obk]

I can hack the const_fn workaround into old borrowck if you really need it

[SimonSapin]

Or you can always use an intermediate constant.

I’d forgotten this was an option! It works for Servo here. However it’s not really discoverable…

[RalfJung]

I’d forgotten this was an option! It works for Servo here. However it’s not really discoverable…

Any suggestions for how to make it more discoverable? The reason promotion is so cautious is that you didn't ask for a static lifetime, and if we promote too aggressively and then CTFE fails we broke your code. With a const, you asked for CTFE and then you can't blame us if there are CTFE failures.

[SimonSapin]

Any suggestions for how to make it more discoverable?

I would say make the error message suggest the intermediate-const-item workaround, but @oli-obk said on IRC that would be hard.

you didn't ask for a static lifetime

Didn’t I? This borrow is immediately passed to a (const fn) function that takes a &'static Foo parameter. The borrow expression itself says &foo() rather than &'static foo() because the latter is not valid expression syntax. And this is in the definition of a static item, so this expression will only ever be evaluated at compile-time.

[oli-obk]

yea, we could totally promote any & to non-cell data in static and const initializers. Implementing this would make the current promotion code even more spaghetti than it already is. We should accept this at some point.

I personally think that we should even accept

static FOO: &u32 = {
    let x = 42;
    &x
};

But that's a different discussion.

[RalfJung]

And this is in the definition of a static item, so this expression will only ever be evaluated at compile-time.

Oh, that context wasn't clear to me. Yeah that's a different game, we can be less careful there.

yea, we could totally promote any & to non-cell data in static and const initializers. Implementing this would make the current promotion code even more spaghetti than it already is. We should accept this at some point.

Let's do this after we kill the HIR-based promotion code and rewrite the other, shall we?^^

[Boscop]

I also wish code like this would work:

struct Foo(u32);

impl Foo {
    const fn new(x: u32) -> Self {
        Foo(x)
    }
}

fn bar() -> &'static Foo {
    &Foo(42) // works because of rvalue promotion
}

fn baz() -> &'static Foo {
    &Foo::new(42) // error: temporary value does not live long enough
}

Because right now I have to create a lot of intermediate constants..

@RalfJung

The reason promotion is so cautious is that you didn't ask for a static lifetime, and if we promote too aggressively and then CTFE fails we broke your code. With a const, you asked for CTFE and then you can't blame us if there are CTFE failures.

Why would CTFE fail (after upgrading the nightly) for expressions that it used to work for (in the previous nightly)? Won't the set of expressions that CTFE works for just expand over time?

[RalfJung]

Why would CTFE fail (after upgrading the nightly) for expressions that it used to work for (in the previous nightly)? Won't the set of expressions that CTFE works for just expand over time?

Imagine we did promotion for a const fn from some library. Now you update the library, and the function changed to do something that will cause a CTFE error.

---

I think @oli-obk's idea here is to introduce some kind of syntax for asking for promotion. Basically, some leight-weight alternative to

{ const FOO = &Foo::new(42); FOO }

Maybe it could be const { &Foo::new(42) }?

If you want to help, the const-eval/CTFE work is happening at https://github.com/rust-rfcs/const-eval/ :)

[oli-obk]

Won't the set of expressions that CTFE works for just expand over time?

This happened because we did in fact expand the set of expression that work in CTFE: we added const fn to stable Rust. Promotion of constants is a very problematic topic with arbitrary code. Imagine

fn baz() -> &'static Foo {
    &unimplemented!()
}

If we promote that, you suddenly get a compile-time error instead of a runtime panic.

There are many similar things that we need to consider. So yes, I'd prefer if we could move to some sort of anonymous constant expressions, but I haven't gotten around to writing an RFC about it.

[abonander]

@oli-obk I think I've found a bug in static rvalue promotion but I'm not sure if it belongs in its own issue:

Given:

struct Value;

impl Value {
    const fn new() -> Self { Value }
}

struct StaticContainer(&'static [Value]);

This doesn't work:

// "Borrowed value does not live long enough" pointing to the slice
static CONTAINER: StaticContainer = StaticContainer(&[Value::new()]);

However, this does:

static VALUES: &'static [Value] = &[Value::new()];
static CONTAINER: StaticContainer = StaticContainer(VALUES);

The former has been accepted on nightly for quite some time now with no additional feature flags but still fails to compile on stable.

[oli-obk]

cc @eddyb

[oli-obk]

Hmm... I just did some testing, and it works on the playground on all channels: https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=8a2ec5bda8c8b08034490c1b1d0f6c76

What's your stable version?

[abonander]

@oli-obk I'm using 2015 edition which explains why it works on stable in 2018 but why then does it work on nightly in 2015?

[oli-obk]

probably because nightly has nll enabled since recently

[oli-obk]

jep, works on beta, too

[abonander]

Nightly has NLL enabled even on 2015 edition?

[oli-obk]

yes, in migration mode, just like 2018

[abonander]

That explains it, thanks.