[nll] cast to raw pointer creates an "enduring" borrow

[nikomatsakis]

The following program (extracted from try_transform_mut v0.1.0) fails to compile with MIR-based borrow check:

#![feature(nll)]
#![allow(unused_variables)]

pub trait TryTransform {
    fn try_transform<F>(self, f: F)
    where
        Self: Sized,
        F: FnOnce(Self);
}

impl<'a, T> TryTransform for &'a mut T {
    fn try_transform<F>(self, f: F)
    where
        Self: Sized,
        F: FnOnce(Self),
    {
        let this: *mut T = self as *mut T;
        f(self);
    }
}

fn main() {
}

The error is:

error[E0499]: cannot borrow `*self` as mutable more than once at a time
  --> src/main.rs:18:11
   |
17 |         let this: *mut T = self as _;
   |                            ---- first mutable borrow occurs here
18 |         f(self);
   |           ^^^^ second mutable borrow occurs here
   |
note: borrowed value must be valid for the lifetime 'a as defined on the impl at 11:6...
  --> src/main.rs:11:6
   |
11 | impl<'a, T> TryTransform for &'a mut T {
   |      ^^

I'm not entirely sure what is going on here, but it seems strange. I would not consider the self as _ to be a "borrow" of self really.

[nikomatsakis]

OK, the problem here is due to the where Self: Sized on the impl. What happens is that we try to prove (at some point) that &'_#9r mut T: Sized (because that is the type of one of our temporaries, and it must be Sized). We find &'a mut T: Sized in the environment, because the impl has where Self: Sized. We therefore conclude that '_#9r = 'a must hold -- this in turn forces our borrow to last for 'a.

If we change to:

#![feature(nll)]
#![allow(unused_variables)]

pub trait TryTransform {
    fn try_transform<F>(self, f: F)
    where
        Self: Sized,
        F: FnOnce(Self);
}

impl<'a, T> TryTransform for &'a mut T {
    fn try_transform<F>(self, f: F)
    where
//        Self: Sized,
        F: FnOnce(Self),
    {
        let this: *mut T = self as *mut T;
        f(self);
    }
}

fn main() {
}

then the code compiles as expected.

I'm actually not sure why the AST code avoids this problem. I guess because it just doesn't happen to try proving Sized for that particular variable. This is basically #21974, a long standing issue without a trivial solution.

Probably the best fix is to special-case Sized in trait handling here -- i.e., in cases like this, we should give priority to the default rules for Sized, rather than relying on the where-clause.

[nikomatsakis]

OK, so one possible fix is to hack in the trait system. Right now, we arbitrarily give where-clauses higher priority than impls. We could tweak this by hacking on

rust/src/librustc/traits/select.rs

Lines 2019 to 2029 in 18925de

	/// Returns true if `victim` should be dropped in favor of
	/// `other`. Generally speaking we will drop duplicate
	/// candidates and prefer where-clause candidates.
	///
	/// See the comment for "SelectionCandidate" for more details.
	fn candidate_should_be_dropped_in_favor_of<'o>(
	&mut self,
	victim: &EvaluatedCandidate<'tcx>,
	other: &EvaluatedCandidate<'tcx>)
	-> bool
	{

The implications are...not entirely obvious, but still, I can't see much harm in modifying that function to give BuiltinBoundCandidate { has_nested: false } the highest possible precedence. After all, it -- like a where clause -- means "unambiguously true", and -- unlike a where-clause -- it won't constrain any regions.

[nikomatsakis]

cc @matthewjasper — I know you're familiar with this code, though you've already got a lot of branches. Thoughts?

[matthewjasper]

@nikomatsakis built in traits used to be more special cased than they are now, so it might be worth checking if I changed something here.

The proposed solution sounds good, although I suspect it might break type inference in some obscure cases.

[nikomatsakis]

@matthewjasper

The proposed solution sounds good, although I suspect it might break type inference in some obscure cases.

Never say never, but the scenarios I can construct in my head are pretty obscure indeed.

The other option is to special case this in MIR borrow check. In particular, we could skip proving T: Sized when it is "obviously" true:

rust/src/librustc_mir/borrow_check/nll/type_check/mod.rs

Lines 902 to 906 in 39e9516

	let trait_ref = ty::TraitRef {
	def_id: tcx.lang_items().sized_trait().unwrap(),
	substs: tcx.mk_substs_trait(place_ty, &[]),
	};
	self.prove_trait_ref(trait_ref, location.interesting());

That is a total hack but seems obviously "fine".

That said, the tweak to select just might also fix #53156, hard to say.

[nikomatsakis]

Zulip topic: https://rust-lang.zulipchat.com/#narrow/stream/122657-wg-nll/topic/.2353123.20.22enduring.22.20borrow