Skip to content

NLL: Implicit reborrow hides error "Cannot move out of Struct with destructor" #52059

@lqd

Description

@lqd
Member

Found while triaging the NLL crater run, in Servo's url crate.

Playground

There's a Drop impl preventing , so the error should at least mention it. But the implicit reborrow self.url hides this error with a '*self.url' does not live long enough one.

Changing the impl to

impl<'a> S<'a> {
    fn finish(self) -> &'a mut String {
        let p = self.url;
        p
    }
}

gives a better error message: self.url cannot move out of type "S<'_>", which implements the "Drop" trait.

Activity

added
A-NLLArea: Non-lexical lifetimes (NLL)
NLL-diagnosticsWorking towards the "diagnostic parity" goal
on Jul 4, 2018
self-assigned this
on Jul 8, 2018
pnkfelix

pnkfelix commented on Sep 11, 2018

@pnkfelix
Member

assigning to self, hoping to investigate this week

self-assigned this
on Sep 11, 2018
pnkfelix

pnkfelix commented on Sep 14, 2018

@pnkfelix
Member

(to be clear, even an explicit reborrow would produce the error message that is being deemed inferior here...)

pnkfelix

pnkfelix commented on Sep 17, 2018

@pnkfelix
Member

To be even more clear: the error here isn't about the illegality of moving out of a struct with a destructor, because the code is not doing any such move.

What the code is doing is borrowing into the interior of a struct with a destructor, and that borrow outlives the lifetime of the struct. Thus the struct's Drop impl might mutably access the borrowed (i.e. aliased) state, violating the rule that all such &mut-accesses are unaliased.


That's not to say that the given error message cannot be improved.

In particular, it might be better if it said something like:

15 |         self.url 
   |         ^^^^^^^^ borrow extends past lifetime of value 
16 |     }
   |     - `self` dropped here while `self.url` still borrowed
   |
   = note: `self` is of type `S<'_>`, which implements `Drop`; thus 
           dropping `self` requires exclusive access to `self.url`.
pnkfelix

pnkfelix commented on Sep 17, 2018

@pnkfelix
Member

Okay I think I have a patch to fix this. And it seems like it improves some of our other diagnostics elsewhere. Will post PR soonish.

added a commit that references this issue on Sep 23, 2018
7714c43
added
I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
and removed
I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
on Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

A-NLLArea: Non-lexical lifetimes (NLL)NLL-diagnosticsWorking towards the "diagnostic parity" goal

Type

No type

Projects

No projects

Relationships

None yet

    Development

    Participants

    @nikomatsakis@pnkfelix@lqd@matthewjasper

    Issue actions

      NLL: Implicit reborrow hides error "Cannot move out of Struct with destructor" · Issue #52059 · rust-lang/rust