[nll] attr-0.1.0 regression

[nikomatsakis]

Found during a crater run

Minimized reproduction

[matthewjasper]

Assigning myself since I've done some work in this locally. But I have a lot going on ATM, so anyone else can feel free to take it.

The issue is the same as #47917, #49822 and #52782 but for enums. I have plans to move all of this checking to happen in/after places_conflict which should also handle this case correctly.

[nikomatsakis]

Further minimized:

#![feature(nll)]
#![allow(dead_code)]

struct S<'a> {
    value: &'a Value,
}

struct Value {
    data: u32,
}

impl<'a> S<'a> {
    fn get(&self) -> Result<&'a mut Value, String> {
        unimplemented!();
    }

    fn at(&self)  {
        let v = self.get();
        if let Ok(Value { ref mut data }) = v {
            let _res : &'a u32 = data;
        }
    }
}

fn main() {
}

[nikomatsakis]

Hmm, looking a bit more closely, I would say that this is a subtle situation. I agree with @matthewjasper that this is basically a "dangly paths" situation (cc #52782). That said, @matthewjasper, I'm not sure I understood the fix you were proposing. =)

Let me lay out what is happening, as best as I understand it:

• First, the call to self.get returns a Result<&'0 mut Value, String>
• Then, the let _res: &'a u32 = ... forces '0: 'a
  • if you remove that type annotation, everything builds
• Finally, we execute Drop(v) -- this is considered an error because there is an outstanding loan of v.as<Ok>.0.
  • This is basically a correct error -- we should not drop things when their interior is borrowed
  • However, in this case, Drop itself does not have a destructor -- the destructor is on String, and of course if String is being dropped, then this is an Err case, and hence we know that the borrow never happened.

[nikomatsakis]

@matthewjasper made a topic on Zulip to discuss

[nikomatsakis]

Removing nomination but we do have to settle on a plan

[Mark-Simulacrum]

@nikomatsakis @matthewjasper @pnkfelix It's not clear to me that this has sufficient momentum or decision-making to land for RC1 -- should this be pushed back to RC2, or is this truly a blocking issue?

[pnkfelix]

(Since not everyone will follow the comments on every linked PR: this can be addressed by PR #54324, but we closed that because we are anticipating a better, broader fix from @matthewjasper , previewed here: master...matthewjasper:better-drop-access )