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

slice::contains with borrowed data #62367

Open
RalfJung opened this issue Jul 4, 2019 · 5 comments
Open

slice::contains with borrowed data #62367

RalfJung opened this issue Jul 4, 2019 · 5 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 4, 2019

HashSet::contains has the following type to allow e.g. searching in a HashSet<String> with an &str:

pub fn contains<Q: ?Sized>(&self, value: &Q) -> bool where
    T: Borrow<Q>,
    Q: Hash + Eq, 

However, slice::contains does not use Borrow, so to search in an &[String] one has to actually allocate a String:

pub fn contains(&self, x: &T) -> bool where
    T: PartialEq<T>, 

Is there a fundamental reason for this, or is this just an omission?

@Mark-Simulacrum
Copy link
Member

I'm fairly certain that this was essentially just a mistake when we implemented it, but we can't change it now as it causes inference failures across the ecosystem. I believe it's been tried multiple times and crater runs are disappointing. It's possible the inference for U: PartialEq or the reverse would also be worse.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 4, 2019

I feared that might be the case. :/

Would it be reasonable to add a contains_bor or so? It's a kludge, but seems better than having to allocate.

@Mark-Simulacrum
Copy link
Member

Probably not worth it -- if you want to skip allocating .iter().any(|e| e == foo) is equivalent to contains.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 4, 2019

Looking at the SliceContains specialization, I am not sure if that's equally efficient for all types?

At the least, the docs should call out this alternative then.

@Mark-Simulacrum
Copy link
Member

Hm, yeah, that might be slower for some cases. I forgot we had specialization internally to optimize for some types. Ideally we'd get around the inference failures somehow, e.g., with a default type param, but that seems unlikely to happen soon :/

Centril added a commit to Centril/rust that referenced this issue Jul 22, 2019
explain how to search in slice without owned data

Cc rust-lang#62367
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 23, 2019
explain how to search in slice without owned data

Cc rust-lang#62367
@Centril Centril added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants