Is an InlineAsm output a potential use that needs to activate a borrow?

[scottmcm]

I added the following FIXME recently: https://github.com/rust-lang/rust/blob/fb245e0/src/librustc_mir/dataflow/impls/borrows.rs#L543-L546

I'll gladly make a PR to change this or remove the fixme, but I don't know enough to decide what should happen.

cc @nikomatsakis maybe?

[nikomatsakis]

Probably it should activate. It'd be nice to make a test of course but I don't think I'm up to that from my phone. On Dec 20, 2017 15:00, scottmcm <notifications@github.com> wrote:I added the following FIXME recently: https://github.com/rust-lang/rust/blob/fb245e0/src/librustc_mir/dataflow/impls/borrows.rs#L543-L546 I'll gladly make a PR to change this or remove the fixme, but I don't know enough to decide what should happen. cc @nikomatsakis maybe? —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[nikomatsakis]

Given the more limited definition of two-phase borrows, I think it's impossible for such a use to activate a borrow?

[pnkfelix]

Re-triaging for #56754. NLL-sound. But since inline ASM is unstable, I'm tentatively tagging as P-medium; there's bigger fish for NLL to fry, IMO.

[matthewjasper]

It also applies only to a version of two-phase borrows that no-longer exists. The current two-phase borrow implementation activates on any use other than a SharedBorrow:

rust/src/librustc_mir/borrow_check/borrow_set.rs

Lines 233 to 296 in ddab10a

	fn visit_local(
	&mut self,
	temp: &Local,
	context: PlaceContext<'tcx>,
	location: Location,
	) {
	if !context.is_use() {
	return;
	}
	// We found a use of some temporary TMP
	// check whether we (earlier) saw a 2-phase borrow like
	//
	// TMP = &mut place
	if let Some(&borrow_index) = self.pending_activations.get(temp) {
	let borrow_data = &mut self.idx_vec[borrow_index];
	// Watch out: the use of TMP in the borrow itself
	// doesn't count as an activation. =)
	if borrow_data.reserve_location == location &&
	context == PlaceContext::MutatingUse(MutatingUseContext::Store)
	{
	return;
	}
	if let TwoPhaseActivation::ActivatedAt(other_location) =
	borrow_data.activation_location {
	span_bug!(
	self.mir.source_info(location).span,
	"found two uses for 2-phase borrow temporary {:?}: \
	{:?} and {:?}",
	temp,
	location,
	other_location,
	);
	}
	// Otherwise, this is the unique later use
	// that we expect.
	borrow_data.activation_location = match context {
	// The use of TMP in a shared borrow does not
	// count as an actual activation.
	PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
	PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
	TwoPhaseActivation::NotActivated,
	_ => {
	// Double check: This borrow is indeed a two-phase borrow (that is,
	// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
	// we've not found any other activations (checked above).
	assert_eq!(
	borrow_data.activation_location,
	TwoPhaseActivation::NotActivated,
	"never found an activation for this borrow!",
	);
	self.activation_map
	.entry(location)
	.or_default()
	.push(borrow_index);
	TwoPhaseActivation::ActivatedAt(location)
	}
	};
	}
	}

[nikomatsakis]

Discussing in NLL meeting. Closing as out of date.