Add "unit struct to normal struct" case to semver.md

[obi1kenobi]

What does this PR try to resolve?

Changing a public unit struct to a normal struct appears to be a breaking change that should require a major version bump: unit structs are always constructible as Foo but normal (i.e. plain) structs are only constructible as Foo {}. The curly braces are required by the compiler, and produce a compilation error and suggestion to add them if they are missing.

The semver page in the reference does not mention anything about unit structs, and as far as I could tell, this case does not seem to be covered in any of the existing entries in the Structs section of the semver page.

This PR adds a new entry in the Structs section of the semver page in the reference, describing this major breaking change and providing an example.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

[ehuss]

I think I would prefer to add a more general rule. I'm pretty sure it is not allowed to change a struct from any of the forms (unit, tuple, or brace). They each have subtle differences. I'm also fairly sure you can't change between other ADTs (union, enum).

https://internals.rust-lang.org/t/pre-rfc-stable-rustdoc-urls/13099 contains a long discussion about the possibility of semver compatibility between types. It's been a long while since I've read it, so I don't remember all the details there, but it would be good to review. However, my instinct is that there is no compatibility between changes (although the context of rustdoc URLs may be a little different from Cargo's SemVer guidelines).

I've also been contemplating changing the approach here to only list what is explicitly allowed (at least in terms of signatures), and then all other changes are assumed to be not allowed. I suspect the list of what is not allowed is going to be much longer than what is allowed. I haven't really thought this through much, but it is something that seems like a possibly better direction. Otherwise we'll just keep adding an endless list of rules of what you can't do.

Also, I'm not sure if you've seen it, but I've been keeping a list of changes that haven't been added in #8736. A problem with managing these rules is that they can be very subtle and can require a lot of thought and consideration. I would like to address as many rules as possible, but most of those rules are tricky.

[obi1kenobi]

I think I would prefer to add a more general rule. I'm pretty sure it is not allowed to change a struct from any of the forms (unit, tuple, or brace). They each have subtle differences. I'm also fairly sure you can't change between other ADTs (union, enum).

Currently, I believe it does not appear that changing a unit struct to a tuple struct must be a breaking change:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c0abcedf80c44a258bd0ed6e11838e64

Of course, we could decide that, despite the conversion being technically allowed by the compiler, a unit -> tuple struct change requires a major version bump regardless. I just wanted to raise this edge case for your consideration.

If the struct has no private fields, any other struct form change is clearly breaking because of differing struct literals rules. However, if the struct is #[non_exhaustive] or has private fields, then struct literals cannot be used and it wasn't clear to me that this would necessarily be a breaking change. A similar argument can be made for enum variants that are #[non_exhaustive] or have private fields. That's why I added this more limited version of the rule. I've never used Rust unions so I haven't thought about them.

https://internals.rust-lang.org/t/pre-rfc-stable-rustdoc-urls/13099 contains a long discussion about the possibility of semver compatibility between types. It's been a long while since I've read it, so I don't remember all the details there, but it would be good to review. However, my instinct is that there is no compatibility between changes (although the context of rustdoc URLs may be a little different from Cargo's SemVer guidelines).

Great link — thanks! It's a bit of a long read and I'm about to go on a trip, so I'll need a bit of time to digest it.

I've also been contemplating changing the approach here to only list what is explicitly allowed (at least in terms of signatures), and then all other changes are assumed to be not allowed. I suspect the list of what is not allowed is going to be much longer than what is allowed. I haven't really thought this through much, but it is something that seems like a possibly better direction. Otherwise we'll just keep adding an endless list of rules of what you can't do.

I understand the hesitation around the very long list of rules. That said, I think having the "explicitly not allowed" approach is overall better for several reasons:

• It's helpful for educational purposes to be able to point users to a docs entry + example that covers why their change is breaking.
• When semver-checkers like my own cargo-semver-checks find a problem, they can link to a specific entry on the reference page and rely on it to explain the problem in more detail rather than having to write ad-hoc explanations of their own.
• In a "list of disallowed things," it's easy to spot a missing rule: just provide an example of a breaking change that isn't covered in the current list of rules. If the list contained only allowed changes, it's much more difficult to argue that an additional kind of change should be allowed. For example, if I had thought something additional should be allowed, I would have assumed that I'm missing something and "there must be a good reason that I'm not seeing," so I never would have opened PRs like this one.

It's also much easier to create a good end-user experience when building semver-checkers (again, like my own cargo-semver-checks) if such tools can look for individual violations of very simple rules (e.g. "don't rename pub structs") rather than checking for an allow-list of rules that is shorter but where the rules are more complex.

The allow-list approach requires much more "understanding" of what the user's change is in order to generate good error messages, and comes with a bigger UX risk: "Is this really a breaking change or did the checker misunderstand my change?"

With simple deny-list rules, it's easier to look at the query that captures the rule and verify that it faithfully encodes the rule because of the relative simplicity. Any violations that query finds can also be easily explained to the user. Since the tool might not cover all rules (since there are many and adding them all to the tool will take time), this approach will make it easier to explain to users which rules are checked by the tool and which are not: one can just show a table of all the semver rules in the reference and show checkmarks next to the ones checked by the tool.

As @oli-obk said on Twitter the other day, we should strongly consider writing the rules in a form amenable to being checked automatically since they are complex and difficult to check by hand. I hope I've convinced you that the deny-list approach is better for checking automatically than the allow-list approach.

I understand that the deny-list approach means more work to write and maintain the semver reference docs. As I'm advocating for that approach, I'm also happy to share in that burden by writing and reviewing new additions to the semver reference docs.

Also, I'm not sure if you've seen it, but I've been keeping a list of changes that haven't been added in #8736. A problem with managing these rules is that they can be very subtle and can require a lot of thought and consideration. I would like to address as many rules as possible, but most of those rules are tricky.

That's a fantastic list! I hadn't seen it, thanks for sharing — it will be very useful to me.

[ehuss]

Currently, I believe it does not appear that changing a unit struct to a tuple struct must be a breaking change:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c0abcedf80c44a258bd0ed6e11838e64

This isn't quite right.

    // but both of these ways to create a Bar work fine
    let _a = Bar;
    let _b = Bar();

_a doesn't construct Bar. It assigns to _a a function that constructs Bar (a tuple constructor).

The subtleties are also around which entities exist in various namespaces. All structs place an entity in the type namespace. Tuple structs also place a constructor function in the value namespace. A unit-struct places a const in the value namespace (equivalent to const Foo: Foo = Foo {}; in your example).

---

Thanks for the comments on the negative rules. That does sound like it'll be useful. Hopefully we can wrangle the rules so that they don't become too endless.

[obi1kenobi]

Ah, my bad — thanks.

Do you know if #[non_exhaustive] structs place entries in the value namespace as well? It seems to me like the tuple struct's constructor, and the unit struct's const are either not present or are inaccessible if those structs are #[non_exhaustive], so I'm not sure whether changing from one non-exhaustive kind of struct to another non-exhaustive kind would be breaking something there.

[ehuss]

I believe the tuple constructor becomes private (it removes the pub) when accessed from a remote crate.

EDIT: (I have an issue open to clarify that.)

[obi1kenobi]

I believe the tuple constructor becomes private (it removes the pub) when accessed from a remote crate.

EDIT: (I have an issue open to clarify that.)

Once again, great link — thanks :)

Removing the pub makes sense. In that case, it seems to me like #[non_exhaustive] structs changing type might be okay from a semver perspective?

Here's my reasoning:

• the constructor / const aren't visible outside the crate,
• struct literals cannot be used from outside the crate because of #[non_exhaustive], and
• ignoring Debug printing different output for plain vs tuple vs unit structs ({ ... } vs ( ... ) vs none), I don't believe there are any other externally-observable differences between struct kinds.

What do you think? Happy to amend the PR with what you think the rule should be.

[ehuss]

Removing the pub makes sense. In that case, it seems to me like #[non_exhaustive] structs changing type might be okay from a semver perspective?

I think only if they have zero public fields. I can't think of a specific case otherwise where that would be a problem.

So, in summary, I think the only safe conversion (aside from struct-tuple-normal-with-private) is: Converting between unit, tuple, and fields struct kinds with #[non_exhaustive] with zero public fields.

If there are any public fields, then it could cause problems with pattern matching.

[bors]

☔ The latest upstream changes (presumably #12339) made this pull request unmergeable. Please resolve the merge conflicts.

[epage]

At this has been waiting-on-author for a couple of years, I'm going to go ahead and close this. We can always pick this up again later.

[obi1kenobi]

Ah, sorry — I did drop the ball on this. Closing is the right call. Thanks, Ed!

I'm not sure I have the right set of skills to write reference-quality docs, and I don't want to take up more of the cargo team's time with reviewing docs PRs and mentoring me toward getting better at it.

As I work on cargo-semver-checks, I'll look to contribute by doing and sharing some of the prep work. I can identify SemVer rules that aren't mentioned in the reference, describe the manner in which they cause breakage, and generally try to supply the prerequisite research needed for someone with better docs-writing skills to put that info into the reference.