More borrow bounds in collections

One thing that has bugged me a few time in the past is that the following code will not compile:

fn main() {
    let args: Vec<String> = std::env::args().skip(1).collect();
    if args.contains("--option") {
        // whatever
    }
}

This fails because contains expects a &String and I just fed it a &str. I find that a bit counter intuitive. VecDeque and LinkedList behave like this as well. It's somewhat inconsistent with the behavior of BTreeSet and HashSet who add a T: Borrow<Q> clause to their argument and for which the above code compile. (Not that it's a good idea here obviously :innocent:)

Given that Borrow is reflexive and has a blanket impl for &T, I think I should be possible to allow the above to work, letting contains accept a broader range of objects and improving semantics as well as consistency with no breaking change.

Thoughts?

3 Likes

I wouldn't be so certain; there are some surprising things that can result in a breaking change! For instance, the increased number of accepted types may introduce type-inference ambiguities, as was found in this crater run for a change which generalizes *Map::entry to accept borrows.

Basically type inference is the only way this could break, but we're willing to accept type inference breaking some time.

I'm a bit worried about the slippery slope we've been setting down where we're increasingly replacing reference arguments with generic AsRef types and so on. If we want coercive semantics for these types, we should just make them coerce instead of creating increasingly abstract and unclear type signatures for common std functions.

3 Likes

@ExpHP ah indeed, type inference could break here. Given the ubiquity of Vec it's possible we'd have more than we'd like. On the other hand contains() has immutable access. Unlike entry() it should only be used on collections that already contain elements so I wouldn't expect the type-inference to rely on it too often (but I might be wrong). It might be different for other methods though.

@withoutboats I agree that the signature would become more difficult to read and I honestly don't like that either. That's the usual debate explicit vs. implicit...

Could we consider something else? Would there be a downside to opening the PartialEq bound to other types since that would at least solve the increased complexity in the type signature:

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

Relevant https://github.com/rust-lang/rust/pull/37761

Thanks. I did look for something similar in the issues but couldn't find it at the time.