Tuples containing references no longer pass PartialEq bounds check for all lifetimes of the other reference

[smwoj]

Code

I tried this code:

trait T {
    type Key<'a>: for<'b> PartialEq<Self::Key<'b>>
    where
        Self: 'a;
}

impl T for () {
    type Key<'a> = (&'a str, u32);
}

It used to compile on 1.69, so I expected it to compile under 1.70, but it failed with the following error

error: implementation of `PartialEq` is not general enough
 --> src/lib.rs:8:20
  |
8 |     type Key<'a> = (&'a str, u32);
  |                    ^^^^^^^^^^^^^^ implementation of `PartialEq` is not general enough
  |
  = note: `(&'a str, u32)` must implement `PartialEq<(&'0 str, u32)>`, for any lifetime `'0`...
  = note: ...but it actually implements `PartialEq`

The same happens for structs that contain a reference and derive PartialEq.

(godbolt link)

Version it worked on

1.69

Version with regression

1.70

rustc --version --verbose:

rustc 1.70.0 (90c541806 2023-05-31)
binary: rustc
commit-hash: 90c541806f23a127002de5b4038be731ba1458ca
commit-date: 2023-05-31
host: x86_64-unknown-linux-gnu
release: 1.70.0
LLVM version: 16.0.2

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

[compiler-errors]

searched nightlies: from nightly-2023-03-01 to nightly-2023-06-21
regressed nightly: nightly-2023-03-29
searched commit range: 2036fdd...478cbb4
regressed commit: 478cbb4

bisected with cargo-bisect-rustc v0.6.5

Host triple: aarch64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2023-03-01 

---

This probably regressed in #109470.

[compiler-errors]

I don't know if this predicate should have held in the first place 🤔

fn test<'a>() where (&'a str, u32): for<'b> PartialEq<(&'b str, u32)> {}

fn main() {
    test();
}

error[E0277]: can't compare `(&str, u32)` with `(&'b str, u32)`
 --> <source>:4:5
  |
4 |     test();
  |     ^^^^ no implementation for `(&str, u32) == (&'b str, u32)`
  |
  = help: the trait `for<'b> PartialEq<(&'b str, u32)>` is not implemented for `(&str, u32)`

I think code demonstrates that (&str, u32): for<'b> PartialEq<(&'b str, u32)> doesn't hold -- the lifetimes have to be equal as a consequence of the PartialEq implementation for tuples

[compiler-errors]

The previous behavior is unsound, demonstrated by this code which passed in 1.69 and fails in 1.70:

trait Lengthen<T> {
    fn lengthen(self) -> T;
}

impl<'a> Lengthen<&'a str> for &'a str {
    fn lengthen(self) -> &'a str { self }
}

trait Gat {
    type Gat<'a>: for<'b> Lengthen<Self::Gat<'b>>;

    fn lengthen(s: Self::Gat<'_>) -> Self::Gat<'static> {
        s.lengthen()
    }
}

impl Gat for () {
    type Gat<'a> = &'a str;
}

fn main() {
    let s = "hello, garbage".to_string();
    let borrow: &'static str = <() as Gat>::lengthen(&s);
    drop(s);

    println!("{borrow}");
}

[BoxyUwU]

fwiw this could possible pass if the PartialEq impl for tuples in std was impl<T, U> PartialEq<(T,)> for (U,) where U: PartialEq<T> { instead of impl<T: PartialEq> PartialEq for (T,) {. But that won't fix the general cause of breakage which is that code was being accepted without trait bounds holding which is unsound, and now those bounds do have to hold which cannot be fixed for all breakages.

[lcnr]

I think there are two actionable steps there:

• to fix this concrete breakage, and because it is strictly more powerful, we can try to change the PartialEq impls in core to impl<T, U> PartialEq<(T,)> for (U,) where U: PartialEq<T> as stated by @BoxyUwU. this may worsen inference but feels generally worthwhile to me.
• Correctly substitute GAT's type used in normalize_param_env in check_type_bounds #109470 does not mention that it fixes an unsoundness, we should add the example by @compiler-errors as a test

[compiler-errors]

(1.) is not feasible, I think. See the crater results for #112899

But yeah, we should add a test for the unsoundness I shared above.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

[workingjubilee]

Breaking code via breaking inference is, strictly speaking, a permitted breakage.
Of course, breaking unsound code is, in a greater sense, also permitted breakage.

[smwoj]

I think there are two actionable steps there:

to fix this concrete breakage, and because it is strictly more powerful, we can try to change the PartialEq impls in core to impl<T, U> PartialEq<(T,)> for (U,) where U: PartialEq as stated by @BoxyUwU. this may worsen inference but feels generally worthwhile to me.
(...)

(1.) is not feasible, I think. See the crater results for #112899

Is there a way to measure the impact of that crater run on the ecosystemt? Breakage in serde or chrono would be surely a more serious problem than a breakage in a toy application and with untrained eye I can't tell if the breakage is disastrous or just a little beyond acceptable.

Improving the PartialEq implementation for tuples strikes me as a "right thing to do" for the standard library. If we can't have it now, would it be possible to change it in a new edition? It would be sad if we were stuck forever with the current implementation when a strictly more powerful one exists.

[workingjubilee]

The stdlib's implementations are largely unrelated to the edition mechanism: the editions mostly introduce syntactic changes, not library changes, with the one blurriness being with macros which interact with syntax.

What really needs to improve is the algorithm for deducing types, then it would be easier for the stdlib to carry more powerful implementations without fear of breaking inference.