librustc: Fix up mutability in method autoderefs if incorrect, and

[pcwalton]

prefer Deref over DerefMut in all other circumstances.

Because the compiler now prefers Deref, this can break code that
looked like:

let mut foo = bar.borrow_mut();
(*foo).call_something_that_requires_mutable_self();

Replace this code with:

let mut foo = bar.baz();
(&mut *foo).call_something_that_requires_mutable_self();

Closes #12825.

[breaking-change]

r? @nikomatsakis

[rust-highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[eddyb]

Can't explicit derefs also be fixed up? Are there issues with that?
I was expecting the early choice to never affect user-visible semantics, maybe only the error messages.

[nikomatsakis]

On Wed, Sep 24, 2014 at 01:35:19AM -0700, Eduard Burtescu wrote:

Can't explicit derefs also be fixed up?

I was expecting to fixup explicit derefs as well.

[nikomatsakis]

@pcwalton based on a read through, this patch seemed pretty small -- mostly unintended side-effects -- so it seems like it's worth trying to fix this the full way, which would mean basically patching up the tables also for the receiver expression (I had envisioned walking the receiver a second time and flipping traits from Deref to DerefMut).

[pcwalton]

@nikomatsakis Recursively too?

[pcwalton]

re-r? @nikomatsakis

[nikomatsakis]

This loop should also consider recurse on field accesses and indexing, I think, and consider autoderefs on those expressions we pass through. This is why expressions like guard.access.pending.remove() require an explicit &mut (which I think they should not).

[pcwalton]

Adjusted the loop above in this way.

[pcwalton]

re-r? @nikomatsakis

[pcwalton]

re-r? @nikomatsakis

[nikomatsakis]

This basically looks good... the only thing that seems to be missing are advanced tests covering scenarios like x.f.get(), (*x).get() and so on. (I know that such things do occur in the code base).

r+ modulo tests.

[pcwalton]

@bors: retry