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

Tracking issue for RFC 3681: Default field values #132162

Open
9 of 17 tasks
estebank opened this issue Oct 25, 2024 · 8 comments
Open
9 of 17 tasks

Tracking issue for RFC 3681: Default field values #132162

estebank opened this issue Oct 25, 2024 · 8 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-default_field_values `#![feature(default_field_values)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Oct 25, 2024

This is a tracking issue for the RFC "3681" (rust-lang/rfcs#3681).
The feature gate for the issue is #![feature(default_field_values)].

Allow struct definitions to provide default values for individual fields and
thereby allowing those to be omitted from initializers. When deriving Default,
the provided values will then be used. For example:

#[derive(Default)]
struct Pet {
    name: Option<String>, // impl Default for Pet will use Default::default() for name
    age: i128 = 42, // impl Default for Pet will use the literal 42 for age
}

let valid = Pet { name: None, .. };
assert_eq!(valid.age, 42);
let default = Pet::default();
assert_eq!(default.age, 42);

let invalid = Pet { .. };

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.
Discussion comments will get marked as off-topic or deleted.
Repeated discussions on the tracking issue may lead to the tracking issue getting locked.

Steps

  • Implement the RFC (cc @rust-lang/compiler)
    • Parse the new syntax
    • Support #[derive(Default)] expansion of structs with default field values
    • Support #[derive(Default)] expansion of struct variants where every field has a default
    • Restrict #[non_exhaustive] on items with default field values
    • Restrict defaults on tuple struct and tuple variants
    • Define visibility of defaulted field values on value construction (S { .. } is allowed if a in struct S { a: () } is not visible?)
    • Restrict S { .. } when S has no fields with default values
    • Lint against explicit impl Default when #[derive(Default)] would be ok
      • Additional lints for iffy cases (maybe belongs in clippy?)
  • Adjust documentation (see instructions on rustc-dev-guide)
    • Add unstable book entry
    • Add to the Reference
    • Mention in The Book
    • Add example to Rust By Example
  • Formatting for new syntax has been added to the Style Guide (nightly-style-procedure)
  • Stabilization PR (see instructions on rustc-dev-guide)

Unresolved Questions

  • What is the right interaction wrt. #[non_exhaustive]? Made mutually exclusive.
  • Customization of behavior wrt. visibility rules (allow user to specify that value can be constructed with .. covering defaulted field that otherwise is not accessible) Following RFC: struct can't be constructed with .. if it covers a private field.
  • Allowing .. on types with no default fields, particularly in unit structs?* Disallowed, can be added later if we find a reason to support that.
  • Allowing the use of field_name: _ to specify the use of the default for that specific field?* Let's not try that, particularly in the face of ~const Default allowing for field_name: Default::default() and field_name: default()
  • Tuple structs and tuple variant support* Let file a subsequent RFC for this.
  • Integration with (potential, at this time) structural records*
  • Integration with (potential, at this time) literal type inference (_ { .. })*
  • Exposing default field values as individual consts in the syntax? (I lean towards "unneeded") If an API needs the default to be expressed, it can be an associated const and use field_name: Self::CONST_DEFAULT, making it accessible
  • Expand support to non-const values? (Personal position is that we shouldn't do that, particularly seeing how powerful const eval is becoming.)

* Decision not needed for stabilization of this feature, can be follow up work.

Implementation history

@estebank estebank added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-default_field_values `#![feature(default_field_values)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 25, 2024
@estebank estebank changed the title Tracking Issue for rust-lang/rfcs#3681 Tracking Issue for rust-lang/rfcs#3681: default field values Oct 25, 2024
@fmease fmease changed the title Tracking Issue for rust-lang/rfcs#3681: default field values Tracking issue for RFC 3681: Default field values Oct 26, 2024
@traviscross traviscross added the B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. label Nov 3, 2024
@Phosphorus-M
Copy link

Hi @estebank , I have a question

Are we considering a syntax like the following?

#[derive(Default)]
struct Pet {
    name: Option<String>, // impl Default for Pet will use Default::default() for name
    age: i128 = 42, // impl Default for Pet will use the literal 42 for age
    foo: Foo = Foo { something: 10 },
}

#[derive(Default)]
struct Foo {
    something: i32 = 2,
}

Something like struct inception as default or overwriting the default implementation.

It's just a question, I don't have the intention to annoying, the RFC is really cool!

@estebank
Copy link
Contributor Author

@Phosphorus-M that would indeed work, <Pet as Default>::default().name would be None and <Pet as Default>::default().foo.something would be 10. You just wouldn't be able to write Pet { .. } without specifying Pet { name: None, .. }. Also, the restriction that the default is a const might restrict what you can express (like initializing an Arc?), but your example would work as expected.

@SUPERCILEX

This comment has been minimized.

@jhpratt
Copy link
Member

jhpratt commented Dec 8, 2024

@SUPERCILEX See this section. It will be supported once the implementation is merged.

@SUPERCILEX

This comment has been minimized.

@estebank estebank added B-unstable Blocker: Implemented in the nightly compiler and unstable. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels Dec 10, 2024
@SpiderUnderUrBed

This comment has been minimized.

@estebank

This comment has been minimized.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 16, 2024
…ive, r=<try>

Lint against manual `impl Default` that could have been `derive`d

```
error: `impl Default` that could be derived
  --> $DIR/manual-default-impl-could-be-derived.rs:74:1
   |
LL | / impl Default for G {
LL | |     fn default() -> Self {
LL | |         G {
LL | |             f: F::Unit,
LL | |         }
LL | |     }
LL | | }
   | |_^
   |
help: you don't need to manually `impl Default`, you can derive it
   |
LL ~ #[derive(Default)] struct G {
   |
```

As part of rust-lang#132162/rust-lang/rfcs#3681 we want to lint when default fields values could preclude the need of a manual `impl Default`, but there are already cases where these manual impls could be derived. This PR introduces a new `default_could_be_derived` lint that makes a best effort check of the body of the `Default::default()` implementation to see if all the fields of a single expression in that body are either known to be `Default` already (like an explicit call to `Default::default()`, a `0` literal, or `Option::None` path) or are identified to be equivalent to the field's type's `Default` value (by opportunistically looking at the `Default::default()` body for that field's type).
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jan 10, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 10, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 10, 2025
Rollup merge of rust-lang#134855 - estebank:default-field-values-unstable-docs, r=jieyouxu

Add `default_field_values` entry to unstable book

Tracking issue: rust-lang#132162
RFC: https://github.com/rust-lang/rfcs/blob/master/text/3681-default-field-values.md
github-merge-queue bot pushed a commit to rust-lang/rust-analyzer that referenced this issue Jan 20, 2025
@estebank
Copy link
Contributor Author

estebank commented Jan 22, 2025

@Phosphorus-M I created #135859 to lint against the case you presented earlier. Doing that by accident can lead to confusion, but making it a lint allows for APIs that need it to still be able to enable it and do it.

Edit: I was convinced that the lint shouldn't live in rustc nor be warn-by-default. We should reimplement it in clippy under the pedantic family, as well as figure out how to detect the same situation when it comes to manual impl Default for _ {}.

hantang pushed a commit to qundao/mirror-mergiraf that referenced this issue Jan 23, 2025
Previously, we needed to specify the default values in multiple places:
- the two `default{,_compact}` constructors
- in the binary, as `default_*_name`
- also in the binary, as the default values for flags

This commit removes all those and uses `None`s in there, instead adding methods called `DisplaySettings::<field_name>_or_default` that return `Some` values if they exist, or the _actual_ defaults in case of `None`.

There was another approach that came to mind, but was seen as too cumbersome: make a newtype for each such field, and implement `Default` for them -- this'd requier too much boilerplate and could require exposing the newtypes in a lot of places (most importantly, in the binary).

Ideally, of course, we'd use default field values, but those aren't stabilzed yet.[^1]

This also gets rid of the TODO in `mergiraf solve`, which was addressed back in #74

[^1]: rust-lang/rust#132162

Reviewed-on: https://codeberg.org/mergiraf/mergiraf/pulls/161
Reviewed-by: Antonin Delpeuch <wetneb@noreply.codeberg.org>
Co-authored-by: Ada Alakbarova <ada.alakbarova@proton.me>
Co-committed-by: Ada Alakbarova <ada.alakbarova@proton.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-default_field_values `#![feature(default_field_values)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants