-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Local is copy #68512
Local is copy #68512
Conversation
@@ -174,7 +174,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> { | |||
// We use `NonUseContext::VarDebugInfo` for the base, | |||
// which might not force the base local to memory, | |||
// so we have to do it manually. | |||
self.visit_local(place_ref.local, context, location); | |||
self.visit_local(&place_ref.local, context, location); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also take a Local
instead of &Local
but I'd leave that for a deeper change related to visitors.
@@ -1773,7 +1773,7 @@ rustc_index::newtype_index! { | |||
|
|||
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] | |||
pub struct PlaceRef<'a, 'tcx> { | |||
pub local: &'a Local, | |||
pub local: Local, | |||
pub projection: &'a [PlaceElem<'tcx>], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orthogonal, but the 'a here could be 'tcx, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure, I just did it to try out and I'm getting ...
error[E0597]: `deref` does not live long enough
--> src/librustc_mir/borrow_check/mod.rs:1400:41
|
855 | impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| ---- lifetime `'tcx` defined here
...
1400 | root_place.projection = &deref;
| ^^^^^^ borrowed value does not live long enough
...
1413 | if places_conflict::borrow_conflicts_with_place(
| ____________-
1414 | | self.infcx.tcx,
1415 | | &self.body,
1416 | | place,
... |
1420 | | places_conflict::PlaceConflictBias::Overlap,
1421 | | ) {
| |_________- argument requires that `deref` is borrowed for `'tcx`
...
1433 | }
| - `deref` dropped here while still borrowed
error[E0621]: explicit lifetime required in the type of `issued_borrow`
--> src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs:547:9
|
344 | issued_borrow: &BorrowData<'tcx>,
| ----------------- help: add explicit lifetime `'cx` to the type of `issued_borrow`: `&'cx borrow_check::borrow_set::BorrowData<'tcx>`
...
547 | err
| ^^^ lifetime `'cx` required
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0597, E0621.
For more information about an error, try `rustc --explain E0597`.
error: could not compile `rustc_mir`.
To learn more, run the command again with --verbose.
Didn't pay a lot of attention to be honest, should I fix this in this PR or investigate this on a different one?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no let's do it in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should start by replacing PlaceRef<'_, 'tcx>
with PlaceRef<'tcx, 'tcx>
and see where that breaks down (most things should be able to handle it, although... unsure how useful it is?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was addressed here #69714
@bors r+ |
📌 Commit 3859a47 has been approved by |
Local is copy r? @oli-obk
⌛ Testing commit 3859a47 with merge 5a876001662d48c423f261d4dd619a551d48ec0a... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
Oh heh, yea this would break clippy. Let's hold up on it until the breakage week is done |
3859a47
to
a13a7d7
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r+ |
📌 Commit 3021856 has been approved by |
☀️ Test successful - checks-azure |
📣 Toolstate changed by #68512! Tested on commit 343432a. 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra). |
Tested on commit rust-lang/rust@343432a. Direct link to PR: <rust-lang/rust#68512> 💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq, @rust-lang/infra).
Rustup to rust-lang/rust#68512 changelog: none
r? @oli-obk