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

Indexing Autoref + Borrow = :( #22826

Closed
Gankro opened this Issue Feb 26, 2015 · 11 comments

Comments

Projects
None yet
6 participants
@Gankro
Copy link
Contributor

Gankro commented Feb 26, 2015

use std::collections::HashMap;
fn main(){
    let mut map = HashMap::new();
    map.insert("foo".to_string(), "bar".to_string());

    // Oh hey I can use borrow to do lookup with a string literal!
    println!("{}", map["foo"]);
}

// <anon>:5:20: 5:30 error: the trait `collections::borrow::Borrow<&str>` 
// is not implemented for the type `collections::string::String` [E0277]
// <anon>:5     println!("{}", map["foo"]);
//                             ^~~~~~~~~~

:(

use std::collections::HashMap;
fn main(){
    let mut map = HashMap::new();
    map.insert("foo".to_string(), "bar".to_string());

    // Ok, maybe I can deref to cancel out the autoref?
    println!("{}", map[*"foo"]);
}

// error: internal compiler error: trying to take the 
// sizing type of str, an unsized type

:((((((

These two conveniences are basically cancelling each-other out for the super-natural case of doing lookup with a string literal. Is there something that can be done about this?

@Gankro

This comment has been minimized.

Copy link
Contributor Author

Gankro commented Feb 26, 2015

CC @aturon

@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Feb 26, 2015

The ICE is definitely a bug, which I may have introduced: *"foo" is treated as a constant expression, but constant expressions have no notion of lvalues, so &*expr works for constant expressions iff *expr is sized (and can be treated as an rvalue).
The following works, btw:

use std::collections::HashMap;
fn main(){
    let mut map = HashMap::new();
    map.insert("foo".to_string(), "bar".to_string());

    // Ok, maybe I can deref to cancel out the autoref?
    let key = "foo";
    println!("{}", map[*key]);
}
@Gankro

This comment has been minimized.

Copy link
Contributor Author

Gankro commented Feb 26, 2015

Yeah, that was my work-around, but I think we can all agree that that's pretty awful.

@steveklabnik steveklabnik added the I-ICE label Feb 26, 2015

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 17, 2015

triage: I-nominated

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 17, 2015

(Hm, triage bot seems to be busted.)

@aturon aturon added the I-nominated label Mar 17, 2015

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 17, 2015

Nominating: we need to at least get the * version working before beta. It would be nice if the more natural version worked as well, but that may require more substantial changes to indexing.

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Mar 19, 2015

1.0 beta for addressing the ICE.

its not clear whether we have time for a breaking-change to try to fix the trait.

(and its also not clear whether a breaking-change is the only option for fixing the trait; e.g. there may be some method via deref-coercion or other magic.)

@pnkfelix pnkfelix added this to the 1.0 beta milestone Mar 19, 2015

@pnkfelix pnkfelix added P-medium and removed I-nominated labels Mar 19, 2015

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 20, 2015

Update: @nikomatsakis is looking into changing the traits to work by-value, as the RFC originally intended.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 26, 2015

Now that index operators take arguments by value I believe this bug is actually fixed. Yay!

use std::collections::HashMap;

fn main() {
    let mut m = HashMap::new();
    m.insert("a".to_string(), 2);
    println!("{}", m["a"]);
}
@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 26, 2015

Does the ICE have a separate issue?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 26, 2015

If you do &*"a" it looks like you run into #22894. If you do *"a" you get a nice legit error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.