[Pre-Pre-RFC] The ..Default::default() shorthand is misleading

language design
1

To anyone newly entering this thread: There is now an updated proposal to address the problem motivated in this opening post that works without changing the syntax.

Take a look at the following struct:

struct World {
    tree: Tree,
    lake: Lake,
    mountain: Mountain,
}

A default implementation of that struct might look like this:

impl Default for World {
    fn default() -> Self {
        Self {
            tree: Tree::grow(),
            lake: Default::default(),
            mountain: Default::default(),
        }
    }
}

Naturally, one would like to shorten the two default implementations in there. Luckily, the language offers a neat shorthand for this case:

impl Default for World {
    fn default() -> Self {
        Self {
            tree: Tree::grow(),
            ..Default::default()
        }
    }
}

Now, the tree is initialised with its special method, and the rest by calling default.... Well, or so I thought up to today. The code with ..Default::default() actually recurses endlessly.

I never read the documentation on the ..Default::default() shorthand in detail, and to me it looked just like what I described before. It is actually not easy to google, as even with quotation marks, the phrase rust "..Default::default()" does not make google understand that the latter is supposed to be a search term with, and in turn it yields no related results. So I asked myself, if I would be the only person with that misunderstanding.

Looking at this issue it appears that other people ran into the same problem. It has 4 linked issues (one of them from me) that follow the same pattern, and the authors mostly wish for a warning for the recursion I created above.

However I would like to go further and argue that this notation is not supposed to be part of the Rust language, because it is ambiguous. It is not clear what the ..Default::default() refers to. It could be either the constructed type (Self in this case), or it could be inserted for each field. I would even argue that the notation hints at it being inserted for each field.

  1. The notation is placed inside the struct, next to the fields.
  2. The .. implies a continuation of what happened before, i.e. before fields are initialised one by one, so the notation would continue that.
  3. The notation is placed as far as possible away from the type it actually relates to (Self in this case).

For these reasons, this notation is violating the goal of the Rust language to encourage code to be explicit in its intentions. Consider the following alternative examples of syntax for the same thing:

impl Default for World {
    fn default() -> Self {
        Default::default() but assign {
            tree: Tree::grow(),
        }
    }
}

Now the call to Default::default() is on the top level, so it looks unrelated to the fields, and more like it will actually be called on Self. Going through our arguments from above:

  1. The notation is placed away from the fields, as it does not relate to them.
  2. No dots, nothing that indicates an "etc" anymore.
  3. The notation is now placed at the position of the struct type, hinting that it is related to that as opposed to some of its fields.

Further, we now have a keyword. If someone does not know what it does, they can google it.

So:

  • Do you agree that the notation in itself is misleading if a reader does not know what it does?
  • Do you agree that this is a problem?
  • What do you think about the notation I proposed?
3 Likes
2

While it may be hard to find on Google, you can at least search for it in the book.

1 Like
3

It's misleading because you're thinking of ..Default::default() as a language feature, but it's not.

It's just the .. syntax combined with a call to Default::default() as its argument.

26 Likes
4

FWIW, I like to think* of myself as a reasonably experienced Rust user, but I had no idea that ..Default::default() worked like that. I had the same expectation that @ISibboI did. That's probably because I had completely forgotten about struct update syntax, and indeed I was thinking of ..Default::default() as a language feature.

Anyway, to answer the questions in the OP:

  • Do you agree that the notation in itself is misleading if a reader does not know what it does?

Yes.

  • Do you agree that this is a problem?

Eh, only kinda. At least rustc warns you when doing ..Default::default() within the implementation of Default::default(). I can't think of any situation where this would actually be a problem in the real world (assuming you listen to rustc's warnings).

  • What do you think about the notation I proposed?

Three thoughts: first, the ..Default::default() syntax will be around forever, given it's just using struct update syntax. If struct update syntax used ..<ident> instead of ..<expr> then alternative syntax may be worth suggesting. But since ..<expr> exists we have to accept that the validity of ..Default::default() as a natural consequence.

Second, whether ..Default::default() should be avoided in favor of a new syntax, I don't think so. I personally have a high bar for new syntax in Rust. IMO new syntax needs to justify itself by showing it fixes an issue that cannot be reasonably fixed in any other way. I think rustc's lint regarding infinite recursion sufficiently addresses the potential problems that may arise from using ..Default::default().

Third, as for the specific syntax of <expr> but assign {...}: I don't really love multi-word keywords. And I'd prefer the Default::default() call be last, not first, since presumably the manually-initialized fields are evaluated first (or rather, their expressions are), and Default::default() is called at the end for any remaining fields.

*Emphasis on think, as evidence is often to the contrary...

10 Likes
5

I don't think this needs a new syntax, this works today and does the same thing:

impl Default for World {
    fn default() -> Self {
        let mut world = Default::default();
        world.tree = Tree::grow();
        world
    }
}
6 Likes
6

This seems like a common enough issue that it might be worth adding a compile-time check for unconditional recursion in impl Default (and perhaps some other similar implementations).

9 Likes
7

There already is a compile-time warning. Though perhaps the warning could be improved to have more messaging (or a link) to better explain what ..Default::default() does (and what it doesn't do).

7 Likes
8

I prefer to write T::default() instead of Default::default(), and while I never encountered this situation, this is another place it would help.

Regarding your questions:

I don't know. I always thought of it as the struct update syntax.

Ditto.

Like already said here, the bar for new syntax should be quite high. Honestly, I don't think this confusion justifies a new syntax, especially when it can be avoided by a lint (and I also dislike multi-work keywords).

1 Like
9

Unfortunately, the warning goes away once the expression for tree is no longer just a simple literal.

13 Likes
10

Thanks, in that case I take back what I said about rustc's warning being sufficient.

11

Thank you for all your answers!

That is a good! However I believe that Rust users are nowadays not teached to use the search function of the book, as answers to nearly everything can be found via google. So as an argument in favor of the syntax, I believe that cannot really count.

You actually uncover another problem with .. as a shorthand in this case. Since ..Default::default() is also an open Range. So what we have here is syntax that is overloaded to mean two completely different things depending on context.

But I don't really get the argument here to be honest. At least you should emphasize that "you're thinking" actually means "quite a few people think".

But this is also an argument against the struct update syntax in general, isn't it? Arguably, the struct update syntax first fills in the special fields, and then the remaining ones from the call to Default::default(). But the same can be achieved by something like this:

let tree = Tree::grow();
let mut world = Default::default();
world.tree = tree;

Nothing but a shorthand.

To everyone (@mjbshaw @josh @chrefr) arguing with compile time warnings about infinite recursion, I think that is actually besides the point. First, as @steffahn rightfully pointed out, in the recursing case, the existing compile time warning is actually very limited:

Now one could of course make the compiler spend some more time on the impl Default and related cases, but the problem is more than just about recursion. Look at the following code:

struct World {
    tree: Tree,
    lake: Lake,
    mountain: Mountain,
}

impl Default for World {
    fn default() {
        World {
            tree: Tree::shrink(),
            lake: Lake::empty(),
            mountain: Mountain::sink(),
        }
    }
}

impl Default for Tree {fn default() {Tree::grow()}}
impl Default for Lake {fn default() {Lake::fill()}}
impl Default for Mountain {fn default() {Mountain::shift()}}

fn main() {
    let world = World {
        tree: Tree::grow(),
        ..Default::default()
    };
}

There is no recursion here. Still, the code is misleading, for the same reasons I stated in the opening post. Why would the ..Default::default() statement here mean that the fields are initialised with the default of the surrounding struct, as opposed to their respective defaults?

As @chrefr rightfully said, this situation could be avoided by writing ..World::default() instead of ..Default::default(). In today's Rust this is a problem though, as one of the idioms of Rust is to "not repeat yourself", and writing the same type twice where it could be inferred is clearly violating that rule.

Of course one could say that there are exceptions to this rule, but any exception is easy to forget and makes the language harder to write. Therefore, if the syntax stays the way it is now, there should be at least a lint against writing ..Default::default() in the struct update syntax.

But I actually proposed a syntax change. And I want to highlight the change here, since all argumentations up to now seem to treat my idea as new syntax in the sense of a new feature. But it should not be. It should simply replace the existing struct update syntax with the exact same functionality.

And I believe that

cannot really be an argument against making a syntax change like this. A programming language should not require to be held together by a bunch of lints steering people around common pitfalls, especially if those pitfalls are of purely human nature and can be avoided by rephrasing some code slightly. Surely, deprecating one syntax can only be done in an edition change, but that is exactly what editions are for. The Rust community has acknowledged that they are unable to make decisions that are always perfect and future proof, so they introduced the edition mechanism to correct oversights of the past. We should not forget that this is one of the important strengths of the Rust language that prevent it to enter the backwards-compatibility trap other languages have fallen into.

About the syntax itself: I agree that double keywords without justification are not nice. And I also agree that since the Default::default() in my example is called last, it should not be in front of the other assignments of the struct. The most important part I would like to change is to move the Default::default() out of the struct, to more clearly separate it from the structs members. Putting these together, the design space is quite limited, and looks something like this:

fn main() {
    let world = World {
        tree: Tree::grow(),
    } <SOME KEYWORD> Default::default();
}

Choosing <SOME KEYWORD> as e.g. overwrites would communicate rather well that the Default::default() relates to World as opposed to the missing struct members.

12

Would there be a relatively common use case for the proposed syntax besides Default::default()? If not, then including the entirety of Default::default() in the syntax seems redundant.

Assuming that the intent was to allow arbitrary expressions, the semantics I would assume would be something like the following, (using the initially proposed but assign syntax):

SOME_EXPR but assign {
    tree: Tree::grow(),
}

would translate to the equivalent of:

World {
    tree: Tree::grow(),
    lake: SOME_EXPR,
    mountain: SOME_EXPR,
}

The amount of other expressions with a meaning for multiple fields of a struct seems like it would be quite small.

The first semi-plausible case I can imagine would be something analogous to the following:

struct DefaultToMax {
  foo: u8,
  bar: u8,
  baz: f32,
}

impl Default for DefaultToMax {
    fn default() -> Self {
        u8::MAX but assign {
            baz: f32::INFINITY,
        }
    }
}

And that use case seems niche to me.

I guess folks who like the builder pattern might like to be able to write:

let builder: SomeBuilder = None but assign {
    initial_buffer_size: Some(512),
};

This also brings up the issue of type inference. Outside of when the syntax is used as last expression in a method returning Self, my guess would be that the type of the but assign expression could not be inferred. That suggests that there would be a desire to insert the type into the expression when needed. (I guess the turbofish syntax could be stuck somewhere?)

I haven't completely thought through the details, but I think it would be possible to write an attribute macro with something like the following syntax that that generates the expected Default implementation:

#[derive_default_but_assign {
    tree: Tree::grow(),
}]
struct World {
    tree: Tree,
    lake: Lake,
    mountain: Mountain,
}

That only addresses the problem of needing to write field: Default::default(), multiple times, of course.

13

Yes but that's common for punctuation.

< means "less than" and it also means "begin generic arguments".

& means "bitwise and", but it also means "reference".

* means "multiply", but it also means "dereference".

You're calling "Default::default()". That's a trait function, so you have to do some type deduction to understand which impl you're calling.

Now there can only be one type here (the type of the struct).

It can't be "the type of the field" because then the question would be: which field? You can't have a single expression be deduced to be different types at the same time.

But actually I find writing Default::default() strange if you know the type. Why would you do that? Write World::default instead and then the type is explicit.

Isn't Default::default() also repeating the same word twice?

8 Likes
14

It's not obvious to me why this would be hard to support in the compiler, if it would in fact be hard.

If you have syntax that allows specifying two different parts of the AST with the same set of tokens, just copy the AST nodes to those two places. Saying the same thing another way, why couldn't the proposed syntax just act like a macro and insert the same code in two different places?

I think ..Default::default() in a struct could have meant "set the remaining fields to their respective defaults". Whether that way would have been better than the real semantics of ..Default::default() is a separate question.

There's an unstable feature that allows using just default() to avoid this repetition. That said, I am not arguing that just because something reduces repetition, that necessarily means it is a good idea. It depends on what other costs there are to going that route.

15

I wasn't really talking about how hard it would be to implement in a compiler, I was talking about the semantics for humans. By "you can't have" I really meant "it would be very unusual and surprising semantics that doesn't exist in any other context".

Because there is no ! character in the proposed syntax change, so it doesn't look like a macro.

1 Like
16

Does it work? I just got an error when I tried it:

error[E0282]: type annotations needed
  --> src/main.rs:16:9
   |
15 |         let mut world = Default::default();
   |             --------- consider giving `world` a type
16 |         world.tree = Tree::grow();
   |         ^^^^^ cannot infer type
   |
   = note: type must be known at this point

Also, how can it work? It would still have to call default recursively, right?

17

I agree. This is a situation we need to imrpove, IMO.

No, because this is the struct update syntax. It is not commonly used besides defaults, and perhaps not well-known, but this is what it is. Not what "quite a few people think".

I don't think it's bad to simplify things and think about it as "complete from the default", if your understanding is actually correct. If not, you're simplifying it to the wrong thing.

The idiom of DRY clashes with code clarity here, and I prefer the clarity (and this is besides things like preventing bugs like that, or assisting type inference).

While editions allow us to make backwards-incompatible changes, they're usually limited to things that will have a very little effect on existing code, because we don't want to break too much code. All of the changes done in editions 2018 and 2021 are like that, except for bare_trait_objects and ellipsis_inclusive_range_patterns, which waited two editions.

I'm not saying this is impossible; but it will take a lot of time, probably a lot of time when both features coexist but the recommendation for new code is to use the new feature, then a lot of time when the old feature is deprecated, and only then we will be able to make it a hard error.

One more thing that seems unclear to me, is the meaning of Default::default() but assign World { tree: Tree::grow() } (or whatever), the same as filling the existing fields with Default::default() (like you assumed the ..Default::default() does) or be the same like ..Default::default() just with another syntax? I assumed this is the latter, but the comment from @Ryan1729 says otherwise.

3 Likes
18

I was just addressing what you said, I wouldn't want to presume what other people think -- especially when that thinking is incorrect.

1 Like
19

I am a bit overwhelmed by all the detailed responses now, but I think this one is very important to address.

I think I was not clear enough before. Thank you for pointing this out.

I want to replace the struct update syntax with a different syntax that ultimately does the same thing.

So where nowadays we would write (in correct Rust, not my misunderstanding):

World {
    tree: Tree::grow(),
    ..Default::default()
}

in the future I would like this to be:

World {
    tree: Tree::grow(),
} overwrite Default::default()

Both desugar to:

{
    let tree = Tree::grow();
    let mut world: World = Default::default();
    world.tree = tree;
    world
}
1 Like
20

I didn't try it. Yeah the inference error is unfortunate, though depending how the proposed syntax is implemented it could also have the same problem.

The example in the first post has the same problem, I just translated it to current rust syntax.