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

RFC: Generic conversion traits #529

Merged
merged 3 commits into from
Mar 20, 2015

Conversation

aturon
Copy link
Member

@aturon aturon commented Dec 16, 2014

This RFC proposes several new generic conversion traits. The
motivation is to remove the need for ad hoc conversion traits (like
FromStr, AsSlice, ToSocketAddr, FromError) whose sole role
is for generics bounds. Aside from cutting down on trait
proliferation, centralizing these traits also helps the ecosystem
avoid incompatible ad hoc conversion traits defined downstream from
the types they convert to or from. It also future-proofs against
eventual language features for ergonomic conversion-based overloading.

Rendered

@aturon
Copy link
Member Author

aturon commented Dec 16, 2014

@andrew-d
Copy link

Just out of sheer curiosity, why the cvt_ prefix?

@SimonSapin
Copy link
Contributor

impl Path {
    fn join_path_inner(&self, p: &Path) -> PathBuf { ... }

    pub fn join_path<P: As<Path>>(&self, p: &P) -> PathBuf {
        self.join_path_inner(p.cvt_as())
    }
}

In this very typical pattern, you introduce an "inner" function that takes the converted value, and the public API is a thin wrapper around that. The main reason to do so is to avoid code bloat: given that the generic bound is used only for a conversion that can be done up front, there is no reason to monomorphize the entire function body for each input type.

Writing two functions/methods with mostly-duplicated signatures is a significant ergonomics hit. I’m not convinced this is better than having lots of special-purpose conversion traits like AsPath.

@SimonSapin
Copy link
Contributor

Uh, nevermind. I just realized that having wrapper functions/methods to reduce the amount of monomorphization is orthogonal to whether conversion traits are generic or specific.

@Kimundi
Copy link
Member

Kimundi commented Dec 16, 2014

LGTM, but the story in regard to ad-hoc conversion traits is still unclear to me.

Would there generic impls like this?

impl<T: Show> To<String> for T {
    fn cnvt_to(&self) -> String { format!("{}", self) /* or however the concrete impl of todays ToString looks like */ }
}

trait ToString: To<String> {
    fn to_string(&self) -> String { self.cnvt_to() }
}

impl<T: To<String>> ToString for T {}

And if not, why?

And how does this deal with conversions that do not have 1:1 mappings? (Like a [Ascii] -> String conversion, which can either be result in "foo" or "['f', 'o', 'o']")

@blaenk
Copy link
Contributor

blaenk commented Dec 17, 2014

I said this in IRC, and I'm writing it here for the record:

Using the ~ symbol this way is the most appealing idea for resurrecting that symbol that I've seen so far, especially because it carries the connotation of an approximation of T, or something that acts like or can be treated as a T.

Using this sugar either for this reason and/or for abstract return types (#518) seems like a great idea to me.

// instead of the previously proposed -> impl Iterator<i32> syntax
fn func() -> ~Iterator<i32> { ... }

@Valloric
Copy link

Please don't add more unnecessary abbreviations in APIs! cvt_ is a pointlessly confusing prefix for something that should just be called (I presume) convert_... and if cvt_ actually stands for something else, then you've made my point for me.

Rust APIs already have a ton of these unfriendly, confusing and unreadable names; let's not add more.

@tomjakubowski
Copy link
Contributor

I might be missing something but I don't see how the four proposed conversion traits could replace FromError. This code:

impl FromError<io::IoError> for MyCoolError { ... }

would be replaced, using the Into trait, with:

impl Into<MyCoolError> for io::IoError { ... }

which, unless I misunderstand coherence, could not be written anywhere but the crate where io::IoError was defined. This seems to defeat the purpose of FromError.

I also agree that the meaning of cvt is obscure and convert should be preferred.

@Ericson2314
Copy link
Contributor

Is there any ways to stabilize the to_string method and not the ToString trait? I hope one day be able to call functions with do notation, so to_string etc can just be semi-monomorphized wrappers functions, and like you said we don't want people using a ToString bound anyways.

If not, I prefer the more radical alternative.

@tomjakubowski Interesting point! Perhaps the coherence rules are / should be: as long as at least Self or one trait parameter is defined in the current crate, things are OK.

@rkjnsn
Copy link
Contributor

rkjnsn commented Dec 22, 2014

I agree that the cvt_ prefix is less that pretty. Would it be possible to have the methods be named the same as their traits?

@chris-morgan
Copy link
Member

@rkjnsn: into and to are fine as method names, but as is a keyword and so would need to become as_.

@djrenren
Copy link

it seems to me that what we really need is a way to tap into the coercion system. basically allow defined coercions to be used with the as syntax. I have no idea how that would manifest but it would clean up the syntax at conversion at least.

@aturon
Copy link
Member Author

aturon commented Feb 27, 2015

Hi all, sorry for the very long absence on this thread. (It was put on the backburner for the initial alpha cycle.)

Now that the dust is settling a bit, and we've been continuing to add new ad hoc conversion traits that could as easily use these generic ones, I want to revive this RFC.

Assuming we address the naming (the cvt_ prefix was just a conversation starter), are there major objections to landing this initial set of traits on an #[unstable] basis? My feeling is, it's hard to know exactly how this will work out without trying it, but we should make sure to re-evaluate before making a final #[stable] commitment.

Detailed responses are below.

@andrew-d

Just out of sheer curiosity, why the cvt_ prefix?

Mostly because I hoped it would spur conversation to find a better name :-) Worst case, we go with convert_ prefixes. (Sadly, we can't do as to and into because as is a keyword.)

@Kimundi

Would there generic impls like this? [...]

Yes.

And how does this deal with conversions that do not have 1:1 mappings? (Like a [Ascii] -> String conversion, which can either be result in "foo" or "['f', 'o', 'o']")

This is similar to the problem of implementing PartialOrd on a type with multiple natural orderings; you have to pick the best/most common one, and provide newtypes or adapters (or, in this case, perhaps explicitly convert) to get the alternatives.

@tomjakubowski

I might be missing something but I don't see how the four proposed conversion traits could replace FromError

You are correct: given our coherence rules, From and Into are not equivalent.

In general, I think that there will still be a role for traits like FromError where you want a particular kind of blanket impl and extensibility. As the RFC discusses to some extent, these generic traits are intended only for very straightforward conversion cases, like AsPath, AsOsStr, IntoPathBuf, ToCStr and so on.

@djrenren

it seems to me that what we really need is a way to tap into the coercion system. basically allow defined coercions to be used with the as syntax.

We may want to do this in the long run, yes. Some ideas along these lines are sketched briefly in the RFC, but I think we should start by simply having the traits at all.

@blaenk
Copy link
Contributor

blaenk commented Feb 28, 2015

I think it's good to go in my opinion, but yeah let's not keep cvt_ prefixes. I think it's a shame that we're holding up into, to, and as_mut though just because we can't use as for the As trait. Not sure what we would use for as though, Like and like() are probably a bit too far off. Another one is as_a(), to_a(), into_a(). I don't know, I'm out of ideas. I wouldn't mind much if we wrote out convert_ entirely.

Hey @alexcrichton, perhaps a few default implementations of this for FFI types satisfy some of your ideas in #892?

I think your:

impl Convert<libc::c_int> for bool {
    fn convert(&self) -> libc::c_int { *self as libc::c_int }
}

I think would be something like:

impl Into<libc::c_int> for bool {
    fn cvt_into(self) -> libc::c_int { *self as libc::c_int }
}

This one:

impl<'a, T> Convert<*const T> for &'a T {
    fn convert(&self) -> *const T { *self as *const T }
}

would probably be:

impl<'a, T> To<*const T> for &'a T {
    fn cvt_to(&self) -> *const T { *self as *const T }
}

Note I typed them without checking, so I may be off, but it seems like it can be done with this, though the difference between this and yours is that now you can't use the same method for each, I guess. I think that was part of the benefit? So that you could blindly call it on anything within a macro? I'm reminded of @aturon's conversation about a trait that generalizes over any bound, perhaps that could be created for these to be able to use a single method? Not sure.

@alexcrichton
Copy link
Member

@blaenk that's a good point! I'd have to think about whether these general purpose traits should be used for that specific of a context, but my first impression would be that they would fit nicely. It's true though that a macro-based approach would require everything implement precisely one of the traits (as opposed to a few different ones), but I think it's ok.

```

What's worse, if you need a conversion that works over any lifetime,
*there's no way to specify it*: you can't write something like
Copy link
Member

Choose a reason for hiding this comment

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

This almost works now:

trait Foo<T>: Sized { fn bar(self, other: T) {} }

fn foo<T>() where for<'a> &'a T: Foo<&'a T> {}

fn main() {}

rust-lang/rust#22999

Copy link
Member

Choose a reason for hiding this comment

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

(I also wonder if we could do lifetime elision for where clauses like this, meaning &T: As<&MyType> would just work.)

@japaric
Copy link
Member

japaric commented Mar 5, 2015

@aturon

I have an old branch where I added the As trait to the stdlib and used it instead of traits like Str, AsSlice, AsOsStr, etc. Most of it worked, but I couldn't convert AsPath to As<Path> because of coherence [1], which is a shame because, in my experience, As<Path> is much more common than e.g. As<[T]> or As<OsStr>, and that would imply that I won't be able to write fn frob(p: ~Path>) in the future.

[1] IIRC, the problem was that impl As<Path> for T where T: AsOsStr is forbidden because As is in libcore, and that impl lives in libstd. One possible solution could be replacing the blanket impl with a bunch of concrete impls (one for each As<OsStr> implementor) - definitely not the nicest solution.

@pcwalton
Copy link
Contributor

Another vote for convert.

@aturon
Copy link
Member Author

aturon commented Mar 13, 2015

I've pushed an update with convert_ prefixes, along with a new From trait that works a little differently (because it is intended to replace custom from_XXX constructors). I also added a section about the prelude, recommending all of the proposed traits for inclusion.

}
```

The interaction between

Choose a reason for hiding this comment

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

What is this?

@Gankra
Copy link
Contributor

Gankra commented Mar 19, 2015

I'm getting a bit cagey about are coercion and equivalence story here; we have Deref, Borrow, and As, To, Into, and From now?

@aturon
Copy link
Member Author

aturon commented Mar 20, 2015

Note: some ongoing discussion at a WIP PR.

@aturon
Copy link
Member Author

aturon commented Mar 20, 2015

Another possible choice of method names: as_ref, as_mut, to and into.

@alexcrichton
Copy link
Member

This RFC has fairly broad support in the overall vision, and there are only some lingering concerns in the last few details of this approach. The PR-in-progress is already quite promising and only has a few minor tweaks. As a result I'm going to merge this.

Tracking issue: rust-lang/rust#23567

@alexcrichton alexcrichton merged commit db410af into rust-lang:master Mar 20, 2015
cmhamill added a commit to cmhamill/rust-notmuch that referenced this pull request Mar 25, 2015
@aturon
Copy link
Member Author

aturon commented Mar 27, 2015

Note that the implementation has diverged somewhat from this RFC, based on experiences rolling out. In particular, we moved to AsRef/AsMut,Into, with corresponding method names, and dropped To for the time being as it was causing some coherence problems and wasn't yet playing a useful role. This is a work in progress.

Meanwhile, @reem has pointed out that there's a much cleaner version of the alternative design that allows you to use methods like into with generics:

trait AsRef<T: ?Sized> {
    fn convert_as_ref(&self) -> &T;
}

trait Into<T> {
    fn convert_into(self) -> T;
}

trait From<T> {
    fn from(T) -> Self;
}

trait Convert {
    fn as_ref<T>(&self) -> &T where Self: AsRef<T>;
    fn into<T>(self) -> T where Self: Into<T>;
}

impl<T> Convert for T {
    fn as_ref<T>(&self) -> &T where Self: AsRef<T> {
        self.convert_as_ref()
    }   
    fn into<T>(self) -> T where Self: Into<T> {
        self.convert_into()
    }
}

// Now you can do:

foo.into::<Bar>()
baz.as_ref::<str>()

This is not very much more complex than the current traits (though you do need to understand the double dispatch to make sense of it), but re-opens the door to the generic traits replacing many ad hoc conversion methods. In particular, it may still be possible to avoid the need for :: in at least some circumstances, leaving you with notation like foo.into<String>(), which is quite nice.

Thoughts?

@aturon
Copy link
Member Author

aturon commented Mar 27, 2015

Note, my main concern about this last proposal is that it introduces an additional barrier to understanding for newcomers, and unless we can remove ::, I think it can't fully replace existing ad hoc conversion methods. So the benefits would likely be limited to:

  • Disambiguating when calling these methods on a concrete type. That situation should be rare, however, since in most cases you should prefer calling ad hoc conversions or constructors.
  • Adding extra type information for clarity. But that could likely be done through ascription.

@blaenk
Copy link
Contributor

blaenk commented Mar 27, 2015

Just so we don't forget about the possibility: we could probably create a reflexive Into blanket impl to enable the "accept borrowed or owned, ultimately need owned, if owned is passed then don't do anything; simply take ownership" idiom.

@reem
Copy link

reem commented Mar 27, 2015

I think we make far too big a deal about the :: in ::<> annotations - it's not that bad.

@wycats
Copy link
Contributor

wycats commented Mar 27, 2015

This is not very much more complex than the current traits (though you do need to understand the double dispatch to make sense of it), but re-opens the door to the generic traits replacing many ad hoc conversion methods. In particular, it may still be possible to avoid the need for :: in at least some circumstances, leaving you with notation like foo.into(), which is quite nice.

I like it.

I don't agree with @reem that ::<> is not that bad, but I do agree that even with ::<> it's pretty nice.

@seanmonstar
Copy link
Contributor

I agree with @reem that ::<> isn't that bad. Though, I use it extensively in hyper for headers.get::<ContentLength>() etc.

@aturon
Copy link
Member Author

aturon commented Mar 31, 2015

Wanted to give an update on this:

After much discussion, we've decided to stick with the design that was landed, modulo some tweaks to the blanket implementations. As outlined above, we intend to retain convenience conversion methods, so e.g. there will be a Path::as_os_str method in addition to an impl Path: AsRef<OsStr>. The conventional conversion methods are working quite well, and we are not in a position to fully replace them with generic ones.

Consequently, there is not much benefit to allowing the explicit type instantiation we've been discussing lately in this thread. Since the generic conversion traits are meant for, well, generic contexts, an annotation should almost never be necessary. If it is desired for clarity, type ascription should provide an easy means of providing. OTOH, providing type instantiation would involve substantial extra complexity for the traits (meaning that the methods go through double dispatch), which would further increase the barrier to newcomers. For such a basic concept as conversions, this would be a real problem.

Note that we can always provide methods with explicit type instantiation later, just with different names than the ones used here.

In the stabilization PR, the blanket impls are tweaked a bit so that both From and Into are automatically reflexive, which makes Into bounds in particular much more widely usable (cc @blaenk). Several convenience conversions were added as well.

@gsingh93
Copy link

gsingh93 commented Apr 7, 2015

Is To gone for good or just for now?

@aturon
Copy link
Member Author

aturon commented Apr 9, 2015

@gsingh93

To turned out to be not-so-useful in practice; if you need ownership, it's almost always better to use Into instead. Reference types can use the to-style implementation, but owned types can be passed in without copying.

So, for example, taking Into<String> rather than To<String> means that clients can pass in both &str, &String, and String values; it will all just work, and do the least expensive thing possible.

@blaenk
Copy link
Contributor

blaenk commented Apr 9, 2015

@aturon YESSS!!! Very late response to your cc of me from 9 days ago in
your previous post. So awesome ;) I expect/hope that Into<SomeOwned> will
become a very popular paradigm, especially thanks to the performance it
could provide by doing the least expensive thing while being very flexible.

On Thursday, April 9, 2015, Aaron Turon notifications@github.com wrote:

@gsingh93 https://github.com/gsingh93

To turned out to be not-so-useful in practice; if you need ownership,
it's almost always better to use Into instead. Reference types can use
the to-style implementation, but owned types can be passed in without
copying.

So, for example, taking Into rather than To means that
clients can pass in both &str, &String, and String values; it will all
just work, and do the least expensive thing possible.


Reply to this email directly or view it on GitHub
#529 (comment).

  • Jorge Israel Peña

@gsingh93
Copy link

gsingh93 commented Apr 9, 2015

@aturon That makes sense. Should the RFC be updated to reflect the implementation?

@Centril Centril added A-slice Slice related proposals & ideas A-conversions Conversion related proposals & ideas A-error-handling Proposals relating to error handling. A-net Proposals relating to networking. A-traits-libstd Standard library trait related proposals & ideas labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-conversions Conversion related proposals & ideas A-error-handling Proposals relating to error handling. A-net Proposals relating to networking. A-slice Slice related proposals & ideas A-traits-libstd Standard library trait related proposals & ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet