Skip to content
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

librustc (RFC #34): Implement the new Index and IndexMut traits. #15394

Closed
wants to merge 1 commit into from

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Jul 4, 2014

This will break code that used the old Index trait. Change this code
to use the new Index traits. For reference, here are their signatures:

pub trait Index<Index,Result> {
    fn index<'a>(&'a self, index: &Index) -> &'a Result;
}
pub trait IndexMut<Index,Result> {
    fn index_mut<'a>(&'a mut self, index: &Index) -> &'a mut Result;
}

Closes #6515.

[breaking-change]

r? @nick29581

let ref_ty = ty::ty_fn_ret(monomorphize_type(bcx, method_ty));
Datum::new(val, ref_ty, LvalueExpr)
}
None => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers: This None branch is the same as the code that was removed below.

Copy link
Member

Choose a reason for hiding this comment

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

Would it be at all reasonable for this slice indexing to go via Index and IndexMut (i.e. just be implemented in the libraries)?

@chris-morgan
Copy link
Member

IndexMut, having been added to the prelude, needs to be added to the Vim syntax file.

@@ -563,12 +562,6 @@ pub fn from_fn(len: uint, f: |index: uint| -> bool) -> Bitv {
bitv
}

impl ops::Index<uint,bool> for Bitv {
Copy link
Member

Choose a reason for hiding this comment

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

This can actually work:

static T: bool = true;
static F: bool = false;
impl ops::Index<uint, bool> for Bitv {
    fn index<'a>(&'a self, i: &uint) -> &'a bool {
        if self.get(*i) { &T } else { &F }
    }
}

It's a little weird, but it does allow the bitv[n] syntax.

@nrc
Copy link
Member

nrc commented Jul 4, 2014

LGTM, modulo the questions inline.

@sfackler
Copy link
Member

sfackler commented Jul 4, 2014

FYI, code that used the old index trait most likely can't use the new ones.


let vt = tvec::vec_types(bcx, ty::sequence_element_type(bcx.tcx(), base_datum.ty));
base::maybe_name_value(bcx.ccx(), vt.llunit_size, "unit_sz");
// Translate index expression and cast to a suitable LLVM integer.
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment doesn't apply to this arm?

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 4, 2014

re-r? @nick29581

Fixed the borrow check and added a test.

… traits.

This will break code that used the old `Index` trait. Change this code
to use the new `Index` traits. For reference, here are their signatures:

    pub trait Index<Index,Result> {
        fn index<'a>(&'a self, index: &Index) -> &'a Result;
    }
    pub trait IndexMut<Index,Result> {
        fn index_mut<'a>(&'a mut self, index: &Index) -> &'a mut Result;
    }

Closes rust-lang#6515.

[breaking-change]
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 7, 2014

Updated and addressed @huonw's comments. re-r? @nick29581

bors added a commit that referenced this pull request Jul 7, 2014
This will break code that used the old `Index` trait. Change this code
to use the new `Index` traits. For reference, here are their signatures:

    pub trait Index<Index,Result> {
        fn index<'a>(&'a self, index: &Index) -> &'a Result;
    }
    pub trait IndexMut<Index,Result> {
        fn index_mut<'a>(&'a mut self, index: &Index) -> &'a mut Result;
    }

Closes #6515.

[breaking-change]

r? @nick29581
@bors bors closed this Jul 8, 2014
@pcwalton pcwalton deleted the new-index-traits branch July 8, 2014 00:29
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.

split Index into Index, IndexMut and IndexAssign
6 participants