[WIP] NLL diagnostic rollup #54347
Closed
[WIP] NLL diagnostic rollup #54347
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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
… into issue-54329-nll-diagnostic-rollup
…onflict' into issue-54329-nll-diagnostic-rollup
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
(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.) |
|
okay spastorino's branch landed so this task is officially not useful. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #54329
Successful merges:
&mutaliasing during Drop #54310: pnkfelix:issue-52059-report-borrow-drop-conflictFailed merges: