Git Product home page Git Product logo

Comments (7)

oli-obk avatar oli-obk commented on July 19, 2024 2

Your issue is more fundamental than a few "missing" trait impls. Unfortunately the compiler cannot figure out that there is an impl that could apply, because it doesn't try to coerce after choosing a type for a generic parameter. Your change would allow this one special case, while still not allowing all other kinds of coercion like function item to pointer, closure to function pointer, type to dyn trait, ...

I don't know if this issue is solveable in the trait solver, or if it would require allowing people to write trait bounds using the Unsize traits. I think we should not add individual impls for certain cases without considering the entire problem and inconsistencies it will leave around.

This may also be a breaking change. I can't come up with a test right now, but it seems like it would cause existing code to fail with type annotations required where it worked fine before

from rust.

demurgos avatar demurgos commented on July 19, 2024
  1. From a high-level point of view, I think that it's actually consistent with Rust's approach to API design to discourage such API overloading. I think that foo's signature should accept a plain option. This also the feedback you received on IRLO.

  2. More specifically regarding your request, I don't think this is analogous to #113489. The role of Cow is to manage optional borrowing. Cow behaves somewhat like &T, since [T; N] converts to &[T] it was a bit inconsistent for it to not convert into Cow<[T]>. What you are proposing here is actually a specific instance of the more general case of converting from T to Option<B> where T: Borrow<B> or T: Deref<Target=B>. It is very surprising (at least to me) for such a conversion to do perform both borrowing and Some-wrapping.

from rust.

minghuaw avatar minghuaw commented on July 19, 2024

Regarding consistency, I would argue otherwise with the example below. Isn't it inconsistent that &[1,2,3] is sometimes treated as &[i32] (ie. bar in the example below) and sometimes treated as &[i32; 3] (ie. foo in the example below)?

fn bar<'a>(a: Option<&'a [i32]>>) { }

fn foo<'a>(a: impl Into<Option<&'a [i32]>>) { }

fn main() {
    bar(Some(&[1, 2, 3])); // This works fine
    foo(Some(&[1, 2, 3])); // This would complain
    foo(&[1, 2, 3]); // This would complain as well
}

Isn't it more intuitive that something that can be accepted as Option<&[T]> should also impl Into<Option<&[T]>>.

from rust.

demurgos avatar demurgos commented on July 19, 2024

I see where you're coming from and why it's surprising. I still feel that this is trying to do too much in a single step, but I may also biased against callee-side conversions and prefer to perform them on the caller side. In particular, I avoid relying on impl<T> From<T> for Option<T>.

Given your last example, it may be acceptable for consistency.

from rust.

minghuaw avatar minghuaw commented on July 19, 2024

This is probably not needed in your use cases, but it doesn't mean other people won't need it. Conversion from &[T; N] into &[T] is the only inconsistency I have seen in my use cases so far, so i don't think it's doing too much in a single step.

from rust.

minghuaw avatar minghuaw commented on July 19, 2024

@oli-obk I agree that this is more than just missing some trait impls, which is also why I had been fine with using &[1, 2, 3][..] before. It was really when I saw the #113489 that makes me think something similar could be done for Option<&[T]>. Given my limited knowledge about the rust compiler, I would love to get more insights about the difference for these two.

I tried to find rationale behind the current design choice within the community but failed to get much help. Folks often simply end up suggesting not using impl Into<Option<&[T]>> entirely (like my conversion with @demurgos above). Thus I figured opening an issue and PR would probably get me some more insights from the team.

This may also be a breaking change. I can't come up with a test right now, but it seems like it would cause existing code to fail with type annotations required where it worked fine before

That was my original concern as well, which was why I opened that PR, and I was surprised the PR passed all tests

from rust.

minghuaw avatar minghuaw commented on July 19, 2024

Please correct me if I'm wrong. My understanding, with my limited knowledge on the compiler, is that the current limitation is caused by the compiler failing to coerce &[T; N] into &[T] when the argument is impl Into<Option<&[T]>>, and adding a trait impl is simply putting a bandaid on the problem, and this band-aid would likely introduce breaking change (that is not covered by current tests) and/or make future changes to the trait solver.

And this is different from the Cow<'a, [T]> case because Cow doesn't impl Cow<'a, T>: From<T>.

from rust.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.