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

Add a Default trait. #8438

Closed
wants to merge 2 commits into from
Closed

Add a Default trait. #8438

wants to merge 2 commits into from

Conversation

emberian
Copy link
Member

No description provided.

@emberian
Copy link
Member Author

This is like Zero, but less awfully named, and with no numeric semantics.

@@ -148,7 +148,7 @@ pub mod clone;
pub mod io;
pub mod hash;
pub mod container;

pub mod default;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this file ended up getting added.

@alexcrichton
Copy link
Member

We could probably just transition everything using Zero to using default (and change it do deriving(Default)).

I think that Zero only really makes sense on numeric types.

@brson
Copy link
Contributor

brson commented Aug 10, 2013

This PR doesn't offer any justification for this feature, link to relevant issues that it solves, doesn't convert any code to use it. I'm getting a little wary of all the new minor features being introduced just because we can. Every thing that goes in introduces new maintenance and cognitive burden, and the closer those features are to the core of the language (as this is) the more justification is required.

Let's consider whether we really need both Zero and Default.

@brson
Copy link
Contributor

brson commented Aug 10, 2013

I stand corrected. There is a linked issue.

@brson
Copy link
Contributor

brson commented Aug 10, 2013

What specific types, that don't already implement Zero, do we expect to implement this, and why?

@thestinger
Copy link
Contributor

@brson: The Zero trait implementations on anything that's not a number should just be removed because it doesn't make sense. I'm not really convinced we need traits for Zero and One at all - are there really numeric types implementing one but not the other?

I've often needed a way to construct new empty containers in generic code, and a Default trait might make more sense than adding new to Container. Although, generic static methods are far too painful to use so there isn't likely to be support for replacing existing new implementations.

I definitely think it would be a lesser evil to support TypeName::static_method_from_trait_in_scope(), despite making :: as magical as ..

@emberian
Copy link
Member Author

Ugh I screwed that commit up major, but I'm going to just start work on TWiR rather than fixing it.

@emberian
Copy link
Member Author

@brson anything that has a static method fn new() -> Self should implement this.

@brson
Copy link
Contributor

brson commented Aug 11, 2013

Ok, I'm convinced. Thanks.

@bblum
Copy link
Contributor

bblum commented Aug 12, 2013

Should fn default not simply be called fn new? (In fact why not call the whole trait New?)

Hmm, one note though is that then people would have to use New; in addition to using the type. So it should perhaps be in the prelude.

@brson
Copy link
Contributor

brson commented Aug 12, 2013

The reason I don't really like calling it new is because of the need to call it with the trait namespace. Having to call some of your constructors MyType::new_foo() and some Default::new() is pretty ugly for the non-generic case. Basically I'd only expect to use Default in generic code and it would almost always be a wrapper around MyType::new

@thestinger
Copy link
Contributor

@bblum, @brson: I think the verbosity issue with static methods is a major issue. It's not good that we've decided to duplicate code in every case as a solution (for example, from_str as a function in every number module).

@emberian
Copy link
Member Author

@thestinger @brson @bblum it actually isn't a problem at all, it could be named new. as this example shows:

trait Newable { 
    pub fn new() -> Self;
}

struct Foo;

impl Newable for Foo {
    pub fn new() -> Foo { Foo }
}

impl Foo {
    pub fn new(x: uint) -> Foo { Foo }
}

fn main() {
    printfln!("%?, %?", Newable::new::<Foo>(), Foo::new(42));
}

@thestinger
Copy link
Contributor

That relies on the assumption that we will never do trait-based resolution for :: as we do for .. I don't find the arguments against doing it strong enough to make up for result of having this choice between code duplication and verbosity.

@emberian
Copy link
Member Author

@pcwalton mind chiming in on that?

@pcwalton
Copy link
Contributor

We should just be able to import static methods. I am very opposed to trait resolution on ::. As the only person who understands resolve as it is, allowing trait resolution to work on :: is just way too much magic.

@thestinger
Copy link
Contributor

@cmr: needs to be updated for the visibility modifier changes

@emberian
Copy link
Member Author

@thestinger thanks, fixed.

@catamorphism
Copy link
Contributor

@cmr Needs a rebase.

@sanxiyn
Copy link
Member

sanxiyn commented Aug 19, 2013

Test failure is legitimate. This PR removed Zero impls for strings, and the failed test tests precisely that. deriving-zero test should be updated.

bors added a commit that referenced this pull request Aug 27, 2013
@bors bors closed this Aug 27, 2013
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

Successfully merging this pull request may close these issues.

9 participants