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

Created on 30 Jul 2020 · 13Comments · Source: rust-lang/rust

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;
}

All 13 comments

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

LeSeulArtichaut on 30 Jul 2020

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

rustbot on 30 Jul 2020

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

spastorino on 30 Jul 2020

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 https://github.com/rust-lang/rust/pull/73504 which is a roll-up, likely https://github.com/rust-lang/rust/pull/72280

spastorino on 30 Jul 2020

cc @nbdd0121 in case you want to fix this is a follow-up of https://github.com/rust-lang/rust/pull/72280

spastorino on 30 Jul 2020

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

lcnr on 30 Jul 2020

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

pnkfelix on 30 Jul 2020

Will look into this today.

@rustbot claim

nbdd0121 on 30 Jul 2020

❤1

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 on 30 Jul 2020

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 (https://github.com/nbdd0121/rust/commit/375a6d073b5944fecc7ca48c0e7ff53154180b2c).

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?

nbdd0121 on 30 Jul 2020

👍1

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?

oli-obk on 31 Jul 2020

@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.

nbdd0121 on 31 Jul 2020

👍1

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

rodrimati1992 on 31 Jul 2020

Was this page helpful?

0 / 5 - 0 ratings