Implementing Copy can be a breaking change

[Jules-Bertholet]

I tried this code:

#[derive(Clone)]
struct Foo<'a>(&'a ());

// Uncomment this line and the code stops compiling
//impl Copy for Foo<'static> {}

struct Bar<'a>(Foo<'a>);

fn do_thing<'a>(b: Bar<'a>) {
    drop(b.0);
}

I expected to see this happen: Uncommenting the line in question does not break the code, as implementing Copy should not be a breaking change

Instead, this happened:

error: lifetime may not live long enough
  --> src/lib.rs:10:10
   |
9  | fn do_thing<'a>(b: Bar<'a>) {
   |             -- lifetime `'a` defined here
10 |     drop(b.0);
   |          ^^^ copying this value requires that `'a` must outlive `'static`

Meta

Regressed in 1.42 according to Godbolt.

@rustbot label A-borrow-checker A-lifetimes A-traits T-types regression-from-stable-to-stable

[QuineDot]

Intentionally denied since Rust 1.41.1.

Here's the PR.

[Jules-Bertholet]

Intentionally denied since Rust 1.41.1.

The example from the blogpost has two drops, which of course must require the field type to be Copy, because otherwise you would access a field that has already been moved out of. My example has only one drop, which should not require any such restriction. So it seems the fix PR was overly strict.

[Noratrieb]

Adding a copy impl for a specific lifetime is not something that should ever be done, and a bug in the program.

[BoxyUwU]

Removing the regression labels since this was intentional (if you disagree feel free to re-add)

[Jules-Bertholet]

Removing the regression labels since this was intentional

Looking through discussion on the PR and on Zulip, I haven't found any evidence that this is the case.

@rustbot label regression-from-stable-to-stable

[Jules-Bertholet]

Adding a copy impl for a specific lifetime is […] a bug in the program.

That's would perhaps be a reasonable position to take, but it's not one the language team has ever FCPed AFAIK.

[Noratrieb]

I don't think we need a lang FCP to decide that implementing copy for only some lifetimes is nonsense^^

[Jules-Bertholet]

The documentation of Copy does not say that. Implementing Copy for only some lifetimes may not seem very useful. But most programs in the space of all valid Rust programs are not useful, which does not make them any less valid; and just because you or I cannot think of an obvious use-case, that does not mean that none exists.

[traviscross]

Minimizing further:

#[derive(Clone)]
struct W<T>(T);
impl Copy for W<&'static ()> {}

fn test<'a>(x: W<&'a ()>) {
    let _x = x;
    //~^ ERROR lifetime may not live long enough
    //~| NOTE copying this value requires that `'a` must outlive `'static`
}

This is, I would imagine, a result of the fact that "lifetimes don't participate in trait selection". E.g.:

struct W<T>(T);

trait OnW {
    fn method(&self) {}
}

trait OnWStatic {
    fn method(&self) {}
}

impl<'a> OnW for W<&'a ()> {}
impl OnWStatic for &W<&'static ()> {}

fn test<'a>(x: W<&'a ()>) {
    (&&x).method();
    //~^ ERROR borrowed data escapes outside of function
    //~| NOTE `x` escapes the function body here
    //~| NOTE argument requires that `'a` must outlive `'static`
}

See Trait solving > Selection in the dev guide:

Note that because of how lifetime inference works, it is not possible to give back immediate feedback as to whether a unification or subtype relationship between lifetimes holds or not. Therefore, lifetime matching is not considered during selection. This is reflected in the fact that subregion assignment is infallible. This may yield lifetime constraints that will later be found to be in error (in contrast, the non-lifetime-constraints have already been checked during selection and can never cause an error, though naturally they may lead to other errors downstream).

Given this, programs such as the above seem unlikely to be accepted in Rust for the foreseeable future.

[apiraino]

Going to remove the prioritization label, by reading the comments I don't see an actionable here. But If this is a regression please change my mind.

@rustbot label -I-prioritize -regression-from-stable-to-stable

[Jules-Bertholet]

My understanding is, this issue stems from the fact that moving vs copying a non-Drop type compiles to different MIR, and this choice must be made without lifetime information. However, according to today's Miri, these operations are identical from an opsem perspective (rust-lang/unsafe-code-guidelines#188); so theoretically, we could represent them with the same MIR, and have the borrow checker do a little more work to verify that no use-after-move is present. Is this correct?