[DRAFT] RFC: Default field values #19
Conversation
| } | ||
| ``` | ||
|
|
||
| ## Default fields values are [`const` context]s |
There was a problem hiding this comment.
Why require this? It seems like they could just be functions evaluated in the position they're used. The "constness" of a given field initializer could be inferred to determine whether a particular field's initializer is const. I can see arguments both ways, but the fact that I can immediately come up w/ examples of things I'd want defaults for that wouldn't be const-valid hints to me that we'll want to expand this.
There was a problem hiding this comment.
Also, this could prevent some of the macros given above from being able to remove their custom #[default] attributes, since many (all?) of those attributes allow non-const expressions.
There was a problem hiding this comment.
idea: warn-by-default lint against non-const defaults (at definition site, not at usage)
There was a problem hiding this comment.
Ping-- @Centril did you have a chance to think more about this?
There was a problem hiding this comment.
Super thorough as always, Mr. Centril! I hope you'll excuse my usual pedantry... Overall, I'm a big fan of this RFC, and I'd like to see it accepted without too many changes.
One final note: it may be worth a brief evaluation of the approach of this RFC vs. the derive-builder crate (or an enhanced version of it). Especially with regards to ergonomics and expressiveness. I don't think I saw that here.
| thereby allowing enums to work with `#[derive(Default)]`. | ||
|
|
||
| ```rust | ||
| #[derive(Default)] |
There was a problem hiding this comment.
Could we use #[derive(Default = Field)] instead of the extra attribute? It has the advantages of not letting the programmer annotate multiple variants and fewer attribute, but does require typing the variant name twice.
| Note that while `#[default]` makes field defaults more useful and is useful | ||
| on its own, it is technically orthogonal in that it can be extracted out of | ||
| this RFC if need be. However, since it completes the picture, it is proposed | ||
| together with field defaults. |
There was a problem hiding this comment.
Default is pretty underused in Rust. I think it is either worth soft-deprecating it by not enhancing it further and not recommending its use, or we should figure out what would make it more used (e.g., putting it in the prelude). I feel a bit uncomfortable proposing enhancements without addressing the deeper concern that it is perhaps not the best construct.
There was a problem hiding this comment.
IMO many of the reasons it is underused are addressed by this PR, such as the ability to easily have defaults that aren't the same as the default values of the field types. I think enhancing it in this way provides a clear benefit, and doesn't contribute to any problems with Default today.
| invert the pattern expression and initialize a `Config` like so (15): | ||
|
|
||
| ```rust | ||
| let config = Config { width, height, .. }; |
There was a problem hiding this comment.
This changes the semantics of non_exhaustive from 'adding fields is backwards compatible' to 'adding default fields is backwards compatible', which I think is itself not a backwards compatible change (in some higher order manner, e.g., consider a library which has used non_exhaustive with the intention of later adding a non-default field, they can no longer do so).
There was a problem hiding this comment.
I left a similar comment above, and agree that this feels like a misleading change to #[non_exhaustive]. IMO adding a private but defaulted field in lieu of using #[non_exhaustive] is a fine way to get the semantics proposed by this RFC.
| The constructor functions `new_first_variant(..)` are not provided for you. | ||
| However, it should be possible to tweak `#[derive(new)]` to interact with | ||
| this RFC so that constructor functions are regained if so desired. | ||
|
|
| ``` | ||
|
|
||
| # Motivation | ||
| [motivation]: #motivation |
There was a problem hiding this comment.
Integrate @cramertj's use cases... :)
| ```rust | ||
| let ls_cmd = LaunchCommand { | ||
| cmd: "ls".to_string(), | ||
| }; |
There was a problem hiding this comment.
Shouldn't there be a .. in this and the next example?
|
bump on this-- @Centril do you have time to chat about this soon-ish? |
|
Hey @Centril — mind if I rip some text from this for a formal RFC? I'd like to make a small one for |
|
@jhpratt Sure, go ahead |
Rendered
This is a draft version of an RFC for you to review, before a formal proposal is made for consideration.
cc @cramertj @nrc @alexreg