Breakage related to default field values (RFC 3681)

[obi1kenobi]

RFC link; Tracking issue (not for discussion, just tracking): rust-lang/rust#132162

In short:

#[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
}

// This works and produces:
// `Pet { name: Some(""), age: 42 }`
let _ = Pet { name: Some(String::new()), .. }

// Compilation error: `name` needs to be specified
let _ = Pet { .. }

Desirable SemVer major, deny by default lints:

• default value removed from struct field, where the field continues to exist and the struct is still externally constructible with a literal
  • Witness: constructing the struct with { .. } syntax now requires supplying a value for that field.
  • It doesn't matter if the field is Default, since Default doesn't matter { .. }-style construction.
• default value removed from enum struct variant field, where the field continues to exist and the variant is still constructible with a literal
  • Witness: constructing the variant with a literal now requires supplying a value for that field

Desirable SemVer major, warn by default lints:

• default value changed on a struct field, from one explicitly-specified value to another
  • Reason to warn instead of error: it's possible the intended SemVer contract was "a default exists" rather than some specific value. It's situational, maintainers can promote this to error.
• default value changed on an enum variant struct field, from one explicitly-specified value to another
  • Analogous to above case.
• default value for a field (either struct or enum struct variant) changed from implicit Default::default() to a different, explicitly-specified value
  • Difficult! Need to determine if impl Default on the struct/enum is derived (hand-impls might not use the Default::default() for the field!), and also need to determine what Default::default() would return for the field's type. Default::default() isn't const in general, so we'd probably have to hardcode defaults for common built-in types.

Progress:

• Waiting on RFC implementation in rustc
• Waiting on rustdoc JSON to include field defaults
• Need new schema in trustfall-rustdoc-adapter
• Ready to write lints

[obi1kenobi]

cc @estebank in case any SemVer edge cases I didn't notice jump out at you. No worries if you're too swamped with other work, just wanted to make you aware of this issue and give you the option to take a look! I like this feature and I'd like cargo-semver-checks support for it to be top-notch 😇

[estebank]

Default::default() is not currently const at all (but I quickly prototyped and it does work as expected).

---

Would you have thoughts about changing from one one way of producing the value to another while the value itself doesn't change?

Like

struct S {
    a: usize = 42,
}

to

struct S {
    a: usize = S::A,
}

impl S {
    const A: usize = 42;
}

[obi1kenobi]

Changing only how the value is produced doesn't seem breaking to me. I can't think of a "witness" program that demonstrates the breakage by working fine on one version and breaking on the next.

Is there a case where you think this might be breaking?

[estebank]

Difficult! Need to determine if impl Default on the struct/enum is derived (hand-impls might not use the Default::default() for the field!), and also need to determine what Default::default() would return for the field's type.

You can check if the impl is marked as #[automatically_derived] for the first check (not 100%, as anyone can apply that themselves, but protects against accidents).

Default::default() isn't const in general, so we'd probably have to hardcode defaults for common built-in types.

rust-lang/rust#134628

Is there a case where you think this might be breaking?

No, it was just something that came to mind and I don't know what I don't know :)

[obi1kenobi]

Thank you!

We do use #[automatically_derived] in a couple of lints already, so it's a good idea to use it here as well. I guess the advice is "please don't add #[automatically_derived] by hand, or else it's at your own risk" :)