NLL Diagnostic Review 3: missing indication that a "partial move" has occurred

[davidtwco]

In ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr, the below error should have the label value used here after move changed to value used here after partial move.

rust/src/test/ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr

Lines 17 to 25 in 850fc6a

	error[E0382]: use of moved value: `line2`
	--> $DIR/borrowck-uninit-field-access.rs:44:5
	|
	LL | let _moved = (line2.origin, line2.middle);
	| ------------ value moved here
	LL | line2.consume(); //[ast]~ ERROR use of partially moved value: `line2` [E0382]
	| ^^^^^ value used here after move
	|
	= note: move occurs because `line2.middle` has type `Point`, which does not implement the `Copy` trait

[davidtwco]

Here are some mentoring instructions to get started on this issue. If you're reading this and want to give this issue a go, drop a comment here and feel free to ask for any help or clarification on Zulip.

If this is your first contribution then there are instructions on getting a local build of the compiler available in the rustc guide.

---

These instructions are up to date as of master being 850fc6a4791b3b0ab668f9fb67c35dddd838b01f.

This error is emitted from the cannot_act_on_moved_value function:

rust/src/librustc_mir/util/borrowck_errors.rs

Lines 485 to 494 in 850fc6a

	let err = struct_span_err!(
	self,
	use_span,
	E0382,
	"{} of {}moved value{}{OGN}",
	verb,
	optional_adverb_for_moved,
	moved_path,
	OGN = o
	);

If we grep in the source, we can find that it is invoked in the report_use_of_moved_or_uninitialized function:

rust/src/librustc_mir/borrow_check/error_reporting.rs

Lines 128 to 134 in 850fc6a

	let mut err = self.infcx.tcx.cannot_act_on_moved_value(
	span,
	desired_action.as_noun(),
	msg,
	self.describe_place_with_options(&moved_place, IncludingDowncast(true)),
	Origin::Mir,
	);

Inside that function, we can see where this label is being added:

rust/src/librustc_mir/borrow_check/error_reporting.rs

Lines 184 to 192 in 850fc6a

	if !is_loop_move {
	err.span_label(
	span,
	format!(
	"value {} here after move",
	desired_action.as_verb_in_past_tense()
	),
	);
	}

In this branch, we'll want to check if the previous move was a partial one (that is, the field we're trying to use and can't had one of its fields moved previously and wasn't itself moved entirely).

To do this, we'll want to look at the previous moves of this data - this is already computed by this function here:

rust/src/librustc_mir/borrow_check/error_reporting.rs

Line 69 in 850fc6a

let move_site_vec = self.get_moved_indexes(context, mpi);

move_site_vec is a Vec<MoveSite> (documentation of MoveSite), we'll want to inspect these to determine whether or not it contains a partial move.

For a given MoveSite, we need to work out the path that was moved - the path to some value is represented by a Place and an example of how we can get the Place of a MoveSite is already present in this function we're working on:

rust/src/librustc_mir/borrow_check/error_reporting.rs

Lines 144 to 145 in 850fc6a

	let move_out = self.move_data.moves[(*move_site).moi];
	let moved_place = &self.move_data.move_paths[move_out.path].place;

Given the place that was moved, we want to check if it was a field of the current used_place. This is done by checking if it is a prefix - eg. if we are trying to use some local _1 and it was partially moved previously, then we'd expect the _1 to be a prefix of the Place for the partial move (like _1.0). We can use the is_prefix_of function for this.

After checking if, for any MoveSite, the used_place is a prefix of associated Place (of that MoveSite), then the note can be modified to add the word "partial".

---

If you've got this working then you should run the tests and update any that change correctly and then open a PR with a line saying r? @davidtwco in the description so I'll be assigned to review it.

[Dylan-DPC-zz]

hi @davidtwco. I can work on this

[clintfred]

@davidtwco I am going to take a run at this at the suggestion of @Dylan-DPC on the Rust Discord.

[bartsmykla]

@clintfred how is it going? Have you had some time to work on that issue, maybe? :-)

[clintfred]

@clintfred how is it going? Have you had some time to work on that issue, maybe? :-)

@bartsmykla Yes. As this is my first time working on the compiler, it took some time to get set up and build everything. I am able to build and run tests. I am on to trying to grok David's excellent mentor instructions.
I haven't yet understood how to get a failing test that I can re-run to show progress. ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr is mentioned, but I could use a little guidance there.

[davidtwco]

I haven't yet understood how to get a failing test that I can re-run to show progress. ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr is mentioned, but I could use a little guidance there.

@clintfred You can run x.py test src/test/ui --test-args borrowck-uninit-field-access and that will build the compiler and run that individual test. Feel free to pass any other arguments you've been giving x.py (such as --stage, --keep-stage or -j) too. There's some documentation here on running tests.

Feel free to hop on Zulip and make a new topic if you have questions and someone will get back to you (or just comment here and I'll reply when I see it).