Skip to content

librustc: Improve method autoderef/deref/index behavior more, and enable IndexMut on mutable vectors. #17934

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

Merged
merged 1 commit into from
Oct 16, 2014

Conversation

pcwalton
Copy link
Contributor

librustc: Improve method autoderef/deref/index behavior more, and enable IndexMut on mutable vectors.

This fixes a bug whereby the mutability fixups for method behavior were
not kicking in after autoderef failed to happen at any level. It also
adds support for Index to the fixer-upper.

Closes #12825.

r? @pnkfelix

@@ -453,12 +453,13 @@ impl<T> Index<uint,T> for Vec<T> {
}

// FIXME(#12825) Indexing will always try IndexMut first and that causes issues.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the fixme be removed?

@pnkfelix pnkfelix changed the title librustc: Improve method autoderef/deref/index behavior more, and enable librustc: Improve method autoderef/deref/index behavior more, and enable IndexMut on mutable vectors. Oct 13, 2014
@pnkfelix
Copy link
Member

(i took the liberty of revising your title and description so that hopefully bors will produce a saner result when it merges)

@nikomatsakis
Copy link
Contributor

FYI, the patch looks roughly right to me. Might be worth comparing the set of AST cases to mem-categorization code -- I think it should corresponding to ownership, basically. One thing that seems to be missing are some run-pass (and compile-fail) tests corresponding to these various cases. Best would probably be:

  1. Some run-pass tests that just test that vectors can be used in various scenarios.
  2. Some tests analogous to the deref tests that check that the mut versions of Deref and Index are actually being invoked.
  3. Some compile-fail tests checking that things which don't implement derefmut/indexmut fail when used in improper ways.

Some of these tests may exist from prior patches, of course.

if i != 0 {
match expr.node {
ast::ExprIndex(ref base_expr, ref index_expr) => {
drop(check::try_overloaded_index(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a current reason for the explicit drop here and below, or is that just carry-over from the previous version of the code? These calls to try are returning Option<T>, which is not marked as must_use ...

@pnkfelix
Copy link
Member

@pcwalton looks good, feel free to r=me after addressing the nits.

@pcwalton pcwalton force-pushed the better-autoderef-fixup branch from 406353c to cc859c7 Compare October 13, 2014 20:56
`IndexMut` on mutable vectors.

This fixes a bug whereby the mutability fixups for method behavior were
not kicking in after autoderef failed to happen at any level. It also
adds support for `Index` to the fixer-upper.

Closes rust-lang#12825.
@pcwalton pcwalton force-pushed the better-autoderef-fixup branch from cc859c7 to f7fb387 Compare October 14, 2014 21:45
bors added a commit that referenced this pull request Oct 15, 2014
librustc: Improve method autoderef/deref/index behavior more, and enable IndexMut on mutable vectors.

This fixes a bug whereby the mutability fixups for method behavior were
not kicking in after autoderef failed to happen at any level. It also
adds support for `Index` to the fixer-upper.

Closes #12825.

r? @pnkfelix
@bors bors closed this Oct 16, 2014
@bors bors merged commit f7fb387 into rust-lang:master Oct 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve method lookup auto-deref behavior (subissue of trait reform)
6 participants