Closed ilya-bobyr closed 1 year ago
@rustbot modify labels to +F-default_free_fn
You call out Default::default()
in the description here - but why aren't T::default()
or <$any_type>::default()
good alternatives? They work today, if the Default trait is in scope, and it is in the prelude.
T::default()
is convenient when the compiler can not infer the type. But when it can, T
because redundant. Here is a discussion where this was mentioned as well: https://github.com/rust-lang/rust/pull/73001
FWIW I think it is often still good style to say T::default()
to communicate to the human reader the type being constructed away, which can be declared very far away in the code (even in a different file).
FWIW I think it is often still good style to say
T::default()
to communicate to the human reader the type being constructed away, which can be declared very far away in the code (even in a different file).
I do no think this change takes away that and in certain cases you have to say T::default()
. The change allows you to say default()
in cases where you are currently saying Default::default()
.
You call out
Default::default()
in the description here - but why aren'tT::default()
or<$any_type>::default()
good alternatives? They work today, if the Default trait is in scope, and it is in the prelude.
Default::default()
happens a lot in struct displays, with StructType { ..., StructType::default() }
being as redundant as StructType { ..., Default::default() }
.
A bit late to the party, but why not just a use Default::default()
in the prelude instead ?
That's less code to inline for the optimizer.
A bit late to the party, but why not just a
use Default::default()
in the prelude instead ? That's less code to inline for the optimizer.
I am not sure I understand exactly what are you suggesting. But if the motivation described in the issues summary is not enough, please, read the discussion in https://github.com/rust-lang/rust/pull/73001
Is there a reason why this new free function isn't added in the prelude when the feature default_free_fn
is used?
@robinmoussu Any additions to the prelude need to go through their own RFC. The bar is quite high, to say the least.
What's the status of this feature? I've found that The Book should be updated (Appendix C - Derivable Traits) but not really sure how to proceed.
One thing that I think is worth calling out is that this is putting a free default
function inside the std::default
module. So in order to avoid the stutter complaint of using Default::default()
, it sounds like you'd have to add a use std::default::default;
somewhere in order to use default
. So that stutter is still there in some sense and you have to write an import for it. Granted, this gets rid of the stutter at the call site.
I also want to bring up the T::default()
alternative. In particular, while it of course is not a replacement to avoid the stutter in all cases, I do think it is generally better to use T::default()
in lieu of either a free default
function or Default::default
. So to me, this on its own reduces the incidence of having to write a stuttering Default::default
call, and thus also reduces the importance of adding things to std that allow one to avoid the stutter.
As a more general point, to be honest, I'm not a huge fan of dumping aliases to things into std. I think it invites questions like, "Wait, what's the difference between the free default
function and Default::default
?" And it kind of just becomes another thing that folks need to learn, and another thing where some code will undoubtedly be using Default::default
and other code will just be using the free function, which are expressed as two different things in std's API.
More generally, do we have other aliases like this in std already? I know we have things like std::u64::MAX
and u64::MAX
, but we've deprecated the former. I wonder if it makes more sense to talk about "let's add aliases to std" as a more general policy question.
In #73001, @dtolnay wrote:
When there are a lot (sometimes 100+) of these in a file it's reasonable to want to import
Default::default
, and the suggestions ofZWriteData { key, ..ZWriteData::default() }
orZWriteData { key, ..Default::default() }
orZWriteData { key, ..<_>::default() }
are not better, whether due to repetition or an intimidating amount of punctuation.
To be honest, this seems infrequent enough where it might be reasonable to just define an internal helper function.
Isn't the plan to have std::default::default
in the prelude ?
I'm not aware of that plan. Could you please link to it?
You're right my bad. It was just a reminiscence of some random reddit post.
We discussed this in today's @rust-lang/libs-api meeting. The general sentiment was:
default()
can work even in const contexts. Ping @rust-lang/wg-const-eval; what's the status of being able to say, for a free function like default()
, that it's const
if and only if there's a const impl of Default
? We'd like to know if that'd be possible, even if it's nightly-only. And if it is, we'd like to use that for default()
.default()
, and adding it to the prelude.it's possible and already works (not yet in bootstrap libcore though): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=abd79539fa8900dd7f9e9d657f5f3e9a
But we are not yet confident in whether we can safely (in the stability sense) stabilize a function that uses this feature without also stabilizing something of that feature at the same time.
Are we working on the assumption that something like use core::default::Default::default;
won't be available in the future? Or is it more that we're okay with the potential for a little duplication down the road?
@jhpratt If one day it becomes possible to use
a trait method, then I think we'd still want to export the free function default()
, we'd just turn it into a pub use
. So I don't see any reason we should wait for that.
@oli-obk That looks great. It'd be helpful to know if we could stabilize a function using const_fn_trait_bound
and const_trait_impl
without first stabilizing those two features themselves, but even if that's not the case, the confirmation that this can work is good enough for me to think we should stabilize this.
@rust-lang/libs-api I'm starting an FCP to get consensus on providing the free function default()
, and exporting it from the prelude. (This consensus does not mean we have to stabilize it immediately; we can work with const-eval to figure out if it's possible to export something based on const_fn_trait_bound
and const_trait_impl
, or if we need to wait for those features. For now, I'd just like to confirm consensus for the general plan of providing the function and the prelude re-export.)
@rfcbot merge
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
Oh sure, I just wanted to make sure that this was desired even in that situation. Personally I'm :+1: regardless.
It'd be helpful to know if we could stabilize a function using const_fn_trait_bound and const_trait_impl without first stabilizing those two features themselves, but even if that's not the case, the confirmation that this can work is good enough for me to think we should stabilize this.
This is still fairly early in the experimental stage, so I don't think we should stabilize anything using it just yet.
@RalfJung The primary blocker from our meeting was whether it would be possible, not necessarily doing so right away.
I'm starting an FCP to get consensus on providing the free function
default()
,
I think my general lean is in line with @BurntSushi's comment here and @dtolnay's comment here. I'm hesitant to add alternative ways to invoke the same functionality without strong justification, and I don't believe that "reducing visual noise" is a sufficiently strong justification in this case. Also, given @petrochenkov's comment indicating that it would likely be easy to add support for use Default::default;
to the language and the fact that as far as I can tell we've not ruled that out, other than that nobody has attempted it, I would prefer seeing an attempt to implement support for importing trait methods in general to stabizing this API.
since its been a month and a half and we're still only at 2/6 in favor of merging, I'm going to go ahead and close the FCP, though I will not be starting a new FCP to close the tracking issue because I still think we should work to resolve this issue through trait method imports or at least rule that out as a possibility.
@rfcbot cancel
@yaahc proposal cancelled.
Related discussion on Internals (some people here are proposing a lint to recommend T::Default
over Default::default
, to clear up some confusion related to the functional update syntax, which would make default()
an anti-pattern right out of the gate):
https://internals.rust-lang.org/t/pre-pre-rfc-the-default-default-shorthand-is-misleading/15917
Bevy 0.7 introduced a similar function in their own prelude: https://bevyengine.org/news/bevy-0-7/#default-shorthand, bevyengine/bevy#4071.
Just wanted to say that I very much enjoy using bevy's default()
function. It's fairly common in struct initialisation, so it does make a tangible difference to the enjoyment of rust have such a shorthand.
There are tons of other traits that contains a single method, for example Zero.
Do we want to create free functions for all the traits?
No, just this one. It's more readable than .. Default::default()
I have included the function in my utils.
You can write:
let mut v: Vec<String> = default();
// then modify the vec ...
and get correct type suggestions on the fly.
I like this style personally.
There is really no need for this extra function, just simply add use default::Default::default
to the prelude
Trait methods can't be imported directly
This came up with Bevy uses of a free default()
function: https://github.com/bevyengine/bevy/issues/8879
The lint UnconditionalRecursion
: "function cannot return without recursing" is not raised by using ..default()
Does not raise a warning:
#![feature(default_free_fn)]
use std::default::default;
struct SomeStruct;
impl Default for SomeStruct {
fn default() -> Self {
Self { ..default() }
}
}
Raises a warning:
struct SomeStruct;
impl Default for SomeStruct {
fn default() -> Self {
Self {
..Default::default()
}
}
}
There is one thing that I noticed:
When I initialize a struct Foo
I often start with Foo { ..Foo::default() }
and then jump to the definition of Foo::default
(using rust-analyzer) to see what the defaults are and to decide which fields I want to change. That's possible with Foo::default()
and Default::default()
, but that's not possible with the free default
function (e.g. the one from bevy, it just jumps to definition inside the bevy crate). That's very unfortunate because from a visual perspective I would prefer ..default()
.
That’s ok, IDEs will get smarter. It’s not a reason to not do it.
On Fri, 23 Jun 2023 at 19:12, Jakob Schikowski @.***> wrote:
There is one thing that I noticed:
When I initialize a struct Foo I often start with Foo { ..Foo::default() } and then jump to the definition of Foo::default (using rust-analyzer) to see what the defaults are and to decide which fields I want to change. That's possible with Foo::default() and Default::default(), but that's not possible with the free default function (e.g. the one from bevy, it just jumps to definition inside the bevy crate). That's very unfortunate because from a visual perspective I would prefer ..default().
— Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/73014#issuecomment-1604664198, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEJCCUR3TTMJ4M4S6HFEDXMXMAXANCNFSM4NTHZFEA . You are receiving this because you commented.Message ID: @.***>
We discussed this in the libs-api meeting today. We would prefer to see a language feature to import the Default::default
method directly. The language team seems to be open to this, but someone needs to write up a language proposal and drive the design of this feature.
Since we are unlikely to stabilize the default
function as-is, we would like to remove it from the standard library.
@rfcbot fcp close
Team member @Amanieu has proposed to close this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
We discussed this in the libs-api meeting today. We would prefer to see a language feature to import the
Default::default
method directly. The language team seems to be open to this, but someone needs to write up a language proposal and drive the design of this feature.Since we are unlikely to stabilize the
default
function as-is, we would like to remove it from the standard library.
I am not familiar with the removal process, but if it is just another issue with, essentially, a revert of the original change, I can do that. Though, if someone happens to have a link to an example, I would appreciate it.
Was the consensus to remove it now, or after support for importing trait methods is added to the language?
If someone wants to mentor me in writing a language proposal, I could probably do that as well. As I have not participated in the language processes in the past, it is not very likely I would manage it by myself.
:bell: This is now entering its final comment period, as per the review above. :bell:
The final comment period, with a disposition to close, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
So, the feature has been removed in favor of a language change, but without actually formally proposing the language change? Disappointing.
We discussed this in the libs-api meeting today. We would prefer to see a language feature to import the
Default::default
method directly. The language team seems to be open to this, but someone needs to write up a language proposal and drive the design of this feature.Since we are unlikely to stabilize the
default
function as-is, we would like to remove it from the standard library.@rfcbot fcp close
TL;DR
maybe we can have a language feature (alias) specifically for when creating structs with StructName {}
to support ..
which expands to ..Default::default
.
MyStruct {
a: 1,
b: 2,
..,
}
This makes sense because the language already has this syntax is places such as pattern matching
has there been any followup RFC/tracking issue for an eventual use Trait::method;
syntax?
has there been any followup RFC/tracking issue for an eventual
use Trait::method;
syntax?
For what it's worth, https://github.com/rust-lang/rfcs/issues/1995 is an open issue on this, maybe we could have some discussion there to move things forward.
The feature gate for the issue is
#![feature(default_free_fn)]
.Addition of a free
default()
function to thedefault
module.When constructing a value using
Default::default()
the word "default" is repeated twice.See https://github.com/rust-lang/rust/pull/73001 for additional details, including alternative solutions.
Steps
Default::default()
to usedefault()
where it makes sense.Unresolved Questions
Implementation history