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

Merge ToStr and the Default format #9806

Closed
orenbenkiki opened this issue Oct 11, 2013 · 3 comments
Closed

Merge ToStr and the Default format #9806

orenbenkiki opened this issue Oct 11, 2013 · 3 comments

Comments

@orenbenkiki
Copy link

Now that the Default trait has been added for handling {} format, there are two ways to define how to convert an object to a string: foo.to_str() and format!("{}", foo); that is, the object needs to implement both ToStr and Default.

The proposal is to eliminate this duplication by eliminating ToStr and, instead, implementing to_str() by internally invoking the formatting function. This of course would be a major breaking change to existing code.

An alternative would be to provide a "default implementation" for the Default trait for anything implementing ToStr. This would be a backward-compatible change but would require extending the type system to allow for allowing default implementation of traits (if the type implements some other traits) with the ability to override the implementation for specific types.

@alexcrichton
Copy link
Member

I think that this is tricky to do. Right now Default and ToStr are different semantically in the sense that I don't think that they can be directly merged. What I would be ok with, however, is to do the following:

  • Remove deriving(ToStr), change it to deriving(String) or maybe FmtString. This would add a deriving implementation of the String formatting trait (the :s qualifier
  • Migrate all types using ToStr to FmtString
  • impl<T: fmt::String> ToStr for T

That way nothing else really needs to implement ToStr, the to_str method is just a convenient way of saying format!("{:s}", t).

One of the main things which I would like to avoid is a deriving-like format of the Default trait because I don't think that there's fantastic way to do that. What do you think about these steps?

@orenbenkiki
Copy link
Author

You are right, I was actually mixing issue #9807 into the proposal, and I shouldn't have - they are logically separate. Yes, indeed, if we ignore issue #9807, it makes more sense to merge String or FmtString or whatever it is called with ToStr instead of merging Default, and making to_str() invoke format! using {:s} instead of {}. Thanks for catching this.

@huonw
Copy link
Member

huonw commented Dec 7, 2013

I was talking to @alexcrichton on IRC, and he was ok with:

13:20:17 - acrichto: […] Default => Format, change ToStr to "impl<T: Format> ToStr for T"
13:20:24 - acrichto: I don't want to add to_str() to the formatting code just yet

This would still require removing the deriving(ToStr) mode.

brendanzab added a commit to brendanzab/rust that referenced this issue Jan 4, 2014
This is in preparation for adding a `#[deriving(Format)]` and removing the `ToStr` trait, as discussed in rust-lang#9806.
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 24, 2014
This has been superseded by deriving(Show).

cc rust-lang#9806
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants