Evaluating the results of the poll
I posted a poll on URLO about 3 days ago, trying to see if beginners would intuitively guess the struct update syntax correctly in the case where it is used as ..Default::default().
I specifically asked about the following code example:
struct Tree;
struct Bush {twigs: i32}
struct Forest {
tree: Tree,
bush: Bush,
}
impl Default for Bush {
fn default() -> Self {
Self {
twigs: 2
}
}
}
impl Default for Forest {
fn default() -> Self {
Self {
tree: Tree,
bush: Bush {twigs: 1},
}
}
}
fn main() {
let forest = Forest {
tree: Tree,
..Default::default()
};
println!("{}", forest.bush.twigs);
}
This is where the answers currently stand, where 1 is the correct answer:
I also decided to separete people by their roles in URLO to estimate their knowledge of the language. Unfortunately, due to rounding errors, the numbers don't add up to 100%.
We can see that roughly 80% of all respondents got the question correct, and roughtly 70% of those that say that they "rather learn" from URLO got it correct. This leaves us with roughly 30% of less experienced respondents that got the question wrong. We cannot know if those people just did not know the answer and guessed randomly, or if they guessed wrongly with confidence. However I believe that it shows that there is a certain ambiguity that should be addressed in some way to make Rust code more readable in this specific case.
Summarising a bit
In general people seem to acknowledge that there is a problem with the case I rised, but the general mood seems to be against a syntax change. I myself am now also of the opinion that that would be too much, especially since I cannot provide good evidence for it changing things to the better. That is, I do not have the resources or the necessary background to make a reliable poll about this topic. This makes it impossible to answer if the current syntax is intuitive or not to many people, and therefore there is no good argument for making a big change like this, at least in this thread.
People have also discussed that warnings or lints would be helpful. To give more details here, lets first distiguish the two main cases.
The first case is the potential unconditional recursion case that looks like this:
impl Default for World {
fn default() -> Self {
Self {
tree: Tree::grow(),
..Default::default()
}
}
}
The second case is the one that might produce unexpected values if misunderstood, which I also used for the poll:
fn main() {
let forest = Forest {
tree: Tree,
..Default::default()
};
}
About the potential unconditional recursion, the following things have been said:
- Three people (other than me) have rised issues on github about a missing warning for this case. Their issues can be found via this issue. Them rising these issues can be clearly interpreted as them being in favour of such a warning.
- Seven people (including likes) wish for more compile time checks for this case: and eight people acknowledge that the existing warning works only in very simple cases:
- Six people (including likes) think that the corresponding warning could explain more or link to an explanation:
- Eleven people (including likes, but since the message is longer it is hard to say what the likers actually meant) think that the main focus of a solution should be outside of the compiler or linter (albeit it is not clear if this was said against changing syntax, or also as a general comment):
About the case with unexpected values, not much has been said here. However in the discussion of the poll on URLO, people seem to mostly agree that a lint for this case might be helpful, specifically if I understand correctly people propose the following (original idea seems to be from user ZiCog):
<typename> {
/* ... */,
..Default::default()
}
should produce a linter warning suggesting:
<typename> {
/* ... */,
..<typename>::default()
}
People have also argued that this might be problematic in the case where <typename>::default() is a different function than the first. People have continued to argue that both implementing default() on a type and the Default trait on that same type is not ideal.
The poll itself hints "that there is a certain ambiguity that should be addressed in some way to make Rust code more readable in this specific case," as presented in the first section of this post.
And there is one argument that might relate to either case:
Updating the proposal
Using the hints above, I would now propose the following:
We take over the idea of the lint from URLO. The lint needs to be clever enough to use fully qualified syntax <<typename> as Default>::default() in case the type implements default() as well.
(There is then still a danger in case the implementation of default() on the type happens after writing the code with the struct update syntax, as it would then change the meaning of that code. But that is a general problem whenever using <typename>::default() like that, and despite that risk, there is already a lint that proposes the use of <typename>::default() in general. Also implementing default() on a type in Rust is surely not idiomatic.)
I propose that the lint lives under category clippy::style in the general case. In the recursive case, I propose it to be in clippy::correctness, because I believe that hardly anyone would write a recursive implementation of Default::default(), so anyone who triggers this lint is more likely to have run in the same misunderstanding as I did.
The lint should explain the misunderstanding that can happen when using ..Default::default(), and have at least a link to the respective section in the book, or should explain the syntax with an example on its own.
I will not comment anything about the compiler warning, as it is not required to fix the problem I raised, assuming that this lint gets added. I am not against improving the warning though.
Please tell me:
- Do you believe that this is an adequate way to address the problem at hand?
- Would you propose changes to the lint?