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

NLL elides type behind a borrow (and its Copy-ness) when reporting move error #51190

Closed
pnkfelix opened this issue May 29, 2018 · 2 comments · Fixed by #51247
Closed

NLL elides type behind a borrow (and its Copy-ness) when reporting move error #51190

pnkfelix opened this issue May 29, 2018 · 2 comments · Fixed by #51247
Assignees
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) NLL-diagnostics Working torwads the "diagnostic parity" goal

Comments

@pnkfelix
Copy link
Member

pnkfelix commented May 29, 2018

If you look at the AST-borrowck output for a case like borrowck-vec-pattern-nesting.rs, you see output like this:

error[E0508]: cannot move out of type `[std::boxed::Box<isize>]`, a non-copy slice

but NLL only reports this:

error[E0507]: cannot move out of borrowed content

It is probably useful to include the type behind the borrow (and the fact that it is non-copy) when that is relevant to the move being attempted.

@pnkfelix pnkfelix changed the title NLL elides Copy-ness of type behind a borrow when reporting move error NLL elides type behind a borrow (and its Copy-ness) when reporting move error May 29, 2018
@estebank estebank added the NLL-diagnostics Working torwads the "diagnostic parity" goal label May 29, 2018
@pnkfelix pnkfelix added A-NLL Area: Non Lexical Lifetimes (NLL) WG-compiler-nll labels May 29, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented May 30, 2018

Hmm, interesting; it seems like the MIR dataflow is at least making an attempt to capture the relevant distinctions here, at least based on the code at the following links:

ty::TySlice(_) =>
return Err(MoveError::cannot_move_out_of(
mir.source_info(self.loc).span,
InteriorOfSliceOrArray {
ty: place_ty, is_index: match proj.elem {
ProjectionElem::Index(..) => true,
_ => false
},
})),

IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => {
tcx.cannot_move_out_of_interior_noncopy(span, ty, is_index, origin)
}

So... is that we never actually construct instances of such errors when doing dataflow for MIR-borrowck? (Or is there some deeper issue with the error reporting?) I will look a bit further into this now.

@pnkfelix pnkfelix self-assigned this May 31, 2018
@pnkfelix
Copy link
Member Author

I think I have this figured out. I hope to have a PR up later today.

pnkfelix added a commit to pnkfelix/rust that referenced this issue May 31, 2018
…bout why the move wasn't a copy.

This should address rust-lang#51190.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 5, 2018
…ved-from-behind-borrow, r=nikomatsakis

NLL: report type moved from behind borrow of array/slice

When NLL has illegal move due to borrowed content in an array or slice, provide feedback about why the move wasn't a copy.

Drive by: While comparing the resulting `.nll.stderr` files to their old borrowck variants, I noticed that the test for borrowck-vec-pattern-nesting.rs was not signaling some errors under NLL due to the test assuming lexical lifetimes. So I fixed that too.

Fix rust-lang#51190
pnkfelix added a commit to pnkfelix/rust that referenced this issue Jun 6, 2018
…bout why the move wasn't a copy.

This should address rust-lang#51190.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 6, 2018
…ved-from-behind-borrow, r=nikomatsakis

NLL: report type moved from behind borrow of array/slice

When NLL has illegal move due to borrowed content in an array or slice, provide feedback about why the move wasn't a copy.

Drive by: While comparing the resulting `.nll.stderr` files to their old borrowck variants, I noticed that the test for borrowck-vec-pattern-nesting.rs was not signaling some errors under NLL due to the test assuming lexical lifetimes. So I fixed that too.

Fix rust-lang#51190
bors added a commit that referenced this issue Jun 8, 2018
…hind-borrow, r=nikomatsakis

NLL: report type moved from behind borrow of array/slice

When NLL has illegal move due to borrowed content in an array or slice, provide feedback about why the move wasn't a copy.

Drive by: While comparing the resulting `.nll.stderr` files to their old borrowck variants, I noticed that the test for borrowck-vec-pattern-nesting.rs was not signaling some errors under NLL due to the test assuming lexical lifetimes. So I fixed that too.

Fix #51190
bors added a commit that referenced this issue Jun 8, 2018
…hind-borrow, r=nikomatsakis

NLL: report type moved from behind borrow of array/slice

When NLL has illegal move due to borrowed content in an array or slice, provide feedback about why the move wasn't a copy.

Drive by: While comparing the resulting `.nll.stderr` files to their old borrowck variants, I noticed that the test for borrowck-vec-pattern-nesting.rs was not signaling some errors under NLL due to the test assuming lexical lifetimes. So I fixed that too.

Fix #51190
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) NLL-diagnostics Working torwads the "diagnostic parity" goal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants