Pre-Pre-RFC: Allow exhaustive matching on `#[non_exhaustive]` types from pinned dependencies

I've been thinking about how the #[non_exhaustive] attribute interacts with pinned dependencies (some-crate = "=1.0.4") and couldn't think of a reason to actually enforce the "non-exhaustiveness" in such a case. That is, when forcing a dependency to be exactly a specific version, the only reason for #[non_exhaustive] to exist – making the addition of fields (for structs / enum variants) or variants (enums) not a breaking change – becomes irrelevant.

This also holds for indirect dependencies as long as the entire dependency chain leading to that dependency uses = version requirements.

If the #[non_exhaustive] attribute was updated in this way, there would be a new middle ground between having lots of space for API evolution and some downstream crate wants (or even requirements) like exhaustively matching error conditions.

See also: The non_exhaustive_patterns lint which allows downstream crates to observe upstream addition of fields / variants of non-exhaustive types. This can be combined with version pinning and unreachable!() catch-all match arms to get pretty much the same thing, but in an ugly way.

(Personal) Motivation: In Ruma we currently expose a cargo feature named unstable-exhaustive-types that disables the vast majority of non_exhaustive attributes. This again is a dirty way to get basically the same thing, but it feels wrong and adds noise to the code:

// this is so long,
#[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
// it should just be
#[non_exhaustive]
1 Like

One problem with this is that pinned dependencies is a purely cargo concept, whereas #[non_exhaustive] is a purely rust language concept.

I don't believe there's any precedent at all for cargo versions affecting well-formedness of a program

14 Likes

Sure, but aren't there already a bunch of rustc CLI flags / options that are practically only used through cargo? Cargo already passes a list of external crates to rustc for extern crate statements not to be required and I imagine with the unstable public/private dependencies feature it also tells rustc which crates are public/private deps for the corresponding lint to work. Why not have Cargo also give rustc a list of crates where non-exhaustiveness need not be checked?

1 Like

IIRC crate deps are (always, any edition) passed as --extern crate, and (at some point at least the plan was) when pub/priv deps are turned on, public dependencies are passed as --extern pub:crate (or --pub-extern crate at one point as well).

If pinning information is passed, it'd be another dimension of information to pass about every dep.

I honestly think the correct solution here is to offer a lint for exhaustiveness checking of #[non_exhaustive] enums. Such a lint would warn if the wildcard arm is an unreachable! and not all variants are covered by other arms. (Yes, this would FP if there were domain knowledge that you only accept certain variants.)

cc @dtolnay who is also a proponent of exhaustively matching non exhaustive enums for syn.

Note also that this would break a property of #[non_exhaustive] structs β€” that they're not arbitrarily constructable in downstream code. A non exhaustive struct is semantically identical to adding a _priv: () field.

5 Likes

I can't recall the name off-hand, but I know there was a discussion of implementing that exact lint recently.

I think we should distinguish non-exhaustive struct or enum-variant from non-exhaustive enum. The former is supposed to be equivalent to the addition of an additional private field, at least for structs... enums don't support private fields, so I don't think the behavior should be changed at all. The latter (non-exhaustive enum) might IMO make more sense to consider for a proposal like this thread's OP proposes.

1 Like

Pinned versions are quite unpleasant to deal with in Cargo.

If two crates pin different versions with the same semver-major version, then Cargo will not be able to keep both, and will fail with a resolution error.

So I think until (if ever) Cargo makes pinned versions work nicer, Rust shouldn't add features that encourage pinning versions.

8 Likes

That lint already exists as an unstable feature. I linked to it in the original post.

You are right, I hadn't considered this. That reduces my motivation for making this a thing, since construction via struct literal is currently being done in at least some uses of the unstable-exhaustive-types feature I mentioned.

My name came up above indicating that I might support this, but I definitely do not. @kornel got it right. Anything that would lead to more people putting = into their deps would be disastrous.

The failure mode in kornel's comment (two crates with different = dependencies on the same dep) is not the only source of conflict. There is also the misguided but sadly widespread fad of folks using Dependabot on library projects, which blindly raises ^ requirements for no reason (example). One crate with an = dependency and another crate with a Dependabot-inspired uselessly high ^ dependency would not work together.

If even a small fraction of crates started using this with syn for example, which is a transitive dependency of 47% of all crates right now, then large chunks of crates.​io would just stop working together.

10 Likes

I didn't want to suggest that this would be applicable in cases like syn. I was thinking about cases where = dependencies are already being used or would at least be a reasonable choice already (e.g. dependencies between multiple crates belonging to the same overall project).

1 Like

I don't see any good way you could constrain the proposed feature that would avoid the enormous negative ecosystem impact. Maybe if it only kicks in when both crates in question have the same authors array in Cargo.toml...

The authors field is optional now :man_shrugging:

Realistically, I think the only sensible way this could be applied is if cargo & crates.io add support for publishing workspaces as a whole, as this would effectively create crates that are an implementation detail.

3 Likes

I looked to see if Dependabot had an issue filed for this and it does (recent, June 30 2021): Cargo: support `versioning-strategy: "increase-if-necessary"` Β· Issue #4009 Β· dependabot/dependabot-core Β· GitHub