Lifetime error when indexing with borrowed index in beta but not in stable

[rodrimati1992]

I tried to compile this code:

use std::mem;
use std::ops::{Index, IndexMut};

pub struct FieldMap;

pub fn insert(this: &mut FieldMap, field: &Field<'_>, value: ()) -> () {
    this[field] = value;
}

pub struct Field<'a> {
    x: &'a (),
}

impl<'a> Index<&'a Field<'a>> for FieldMap {
    type Output = ();

    fn index(&self, _: &'a Field<'a>) -> &() {
        unimplemented!()
    }
}

impl<'a> IndexMut<&'a Field<'a>> for FieldMap {
    fn index_mut(&mut self, _: &'a Field<'a>) -> &mut () {
        unimplemented!()
    }
}

I expected the code to compile

Instead, it failed to compile with this lifetime error:

error[E0623]: lifetime mismatch
 --> src/lib.rs:7:5
  |
6 | pub fn insert(this: &mut FieldMap, field: &Field<'_>, value: ()) -> () {
  |                                           ----------
  |                                           |
  |                                           these two types are declared with different lifetimes...
7 |     this[field] = value;
  |     ^^^^^^^^^^^ ...but data from `field` flows into `field` here

Workarounds

If I change the function in these ways it compiles.

https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=ca6b8a2243d7116cb8180bb4300f6edc

pub fn insert<'a,'b,'c>(this: &'a mut FieldMap, field: &'b Field<'c>, value: ()) -> () {
    this[field as &'b Field<'b>] = value;
}

https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=21326b3dcd6c017d0e644ef7eab89769

pub fn insert<'a,'b,'c>(this: &'a mut FieldMap, field: &'b Field<'c>, value: ()) -> () {
    *this.index_mut(field) = value;
}

Meta

This compiles successfully on Stable Rust 1.45.0 back to 1.26.0 (when '_ lifetimes were stabilized)

This fails to compile on these versions(haven't tried previous ones in the same channels):

• 1.46.0-beta.2 (2020-07-23 6f95990)
• 1.47.0-nightly (2020-07-28 a7eff79)

This issue has been assigned to @nbdd0121 via this comment.

[LeSeulArtichaut]

Let’s find the culprit for this. @rustbot ping cleanup

[rustbot]

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

[spastorino]

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[spastorino]

searched nightlies: from nightly-2020-06-19 to nightly-2020-07-11
regressed nightly: nightly-2020-06-20
searched commits: from https://github.com/rust-lang/rust/commit/e55d3f9c5213fe1a25366450127bdff67ad1eca2 to https://github.com/rust-lang/rust/commit/2d8bd9b74dc0cf06d881bac645698ccbcf9d9c5e
regressed commit: https://github.com/rust-lang/rust/commit/72417d84fb51495a4f1d007fb2397a0b2609ab63

Regressed in #73504 which is a roll-up, likely #72280

[spastorino]

cc @nbdd0121 in case you want to fix this is a follow-up of #72280

[lcnr]

The explicit lifetimes are

pub fn insert<'a, 'b>(this: &'a mut FieldMap, field: &'b Field<'a>, value: ()) -> () {
    this[field] = value;
}

edit: the lt of this doesn't matter

[pnkfelix]

assigning to @nikomatsakis but they will probably need to delegate as there time is limited in near future.

[nbdd0121]

Will look into this today.

@rustbot claim

[nbdd0121]

This code compiles in Rust 1.17 but not in Rust 1.18+:

use std::ops::{Index, IndexMut};

pub struct T;
impl T {
    fn test(&mut self) {
    }
}

pub struct FieldMap;

pub fn insert<'a, 'b>(this: &'a mut FieldMap, field: &'b Field<'a>) -> () {
    this[field].test();
}

pub struct Field<'a> {
    x: &'a T,
}

impl<'a> Index<&'a Field<'a>> for FieldMap {
    type Output = T;

    fn index(&self, _: &'a Field<'a>) -> &T {
        unimplemented!()
    }
}

impl<'a> IndexMut<&'a Field<'a>> for FieldMap {
    fn index_mut(&mut self, _: &'a Field<'a>) -> &mut T {
        unimplemented!()
    }
}

#72280 unifies DerefMut/IndexMut to use the "fix-up" path originally used when performing method calls, so it broadens the bug to assignments as well.

[nbdd0121]

Okay, this is an issue near convert_place_op_to_mutable. Just re-infer the index type and re-do the coercion is sufficient to fix the regionck failure (nbdd0121@375a6d0).

However I am not sure why the existing code does not work. This piece of code is last touched in #72068. cc @estebank @oli-obk do you have any clue why the existing approach does not work well with regionck?

[oli-obk]

Hmm.. I would not have expected that code to have to change, since the failure seems to try to make the anonymous lifetime on &mut self in the index_mut method match 'a, which it should not. I think your patch is sound, but touching the "wrong" thing. Basically now we are creating new lifetimes for the index, when I would have thought that the container type's reference's lifetime needs adjustment. So looking at convert_place_op_to_mutable, I'm guessing https://github.com/nbdd0121/rust/blob/375a6d073b5944fecc7ca48c0e7ff53154180b2c/src/librustc_typeck/check/place_op.rs#L308 is taking the wrong region and should instead invent a new one?

[nbdd0121]

@oli-obk This issue is not related to anonymous lifetime actually, if you spell out the lifetime explicitly it's still the same. The root cause has been identified and that line actually is the culprit, a detailed analysis can be found in #74960.

[rodrimati1992]

I added a couple of examples of workarounds that allowed the function to compile, with the same signature.