Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] NLL diagnostic rollup #54347

Closed

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Sep 19, 2018

Fix #54329

Successful merges:

Failed merges:

spastorino and others added 7 commits September 17, 2018 13:38
…at needs exclusive access.

In particular:

 1. Extend `WriteKind::StorageDeadOrDrop` with state to track whether
    we are running a destructor or just freeing backing storage.  (As
    part of this, when we drop a Box<..<Box<T>..> where `T` does not
    need drop, we now signal that the drop of `T` is a kind of storage
    dead rather than a drop.)

 2. When reporting that a value does not live long enough, check if
    we're doing an "interesting" drop, i.e. we aren't just trivally
    freeing the borrowed state, but rather a user-defined dtor will
    run and potentially require exclusive aces to the borrowed state.

 3. Added a new diagnosic to describe the scenario here.
It is worth pointing out that the reason that so few diagnostics are
effected is because of the filter I put in where it only goes down the
new path if the borrowed place is *not* a prefix of the dropped place.

(Without that filter, a *lot* of the tests would need this change, and
it would probably be a net loss for the UX, since you'd see it even in
cases like borrows of generic types where there is no explicit mention
of `Drop`.)
error[E0106]: missing lifetime specifier
 --> /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md:11424:23
  |
9 | fn demo(s: &mut S) -> &mut String { let p = &mut *(*s).data; p }
  |                       ^ expected lifetime parameter
  |
  = help: this function's return type contains a borrowed value, but the signature does not say which one of `s`'s 2 lifetimes it is borrowed from
…onflict' into issue-54329-nll-diagnostic-rollup
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2018
@pnkfelix pnkfelix changed the title NLL diagnostic rollup [WIP] NLL diagnostic rollup Sep 19, 2018
@pnkfelix
Copy link
Member Author

(I marked this WIP because I don't know if its worth keeping without adding some more PR's into it. In particular, the two PR's it rolls up actually merged with no effort.)

@memoryruins memoryruins added the A-NLL Area: Non-lexical lifetimes (NLL) label Sep 19, 2018
@pnkfelix
Copy link
Member Author

okay spastorino's branch landed so this task is officially not useful.

@pnkfelix pnkfelix closed this Sep 20, 2018
@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NLL: rollup a bunch of diagnostic PR's
6 participants