Open smoothdeveloper opened 5 years ago
Great idea!
Another con to this is that you can achieve this today by setting warnings as errors, though I can see someone wanting compile errors for failing to be exhaustive while not wanting an error on all warnings.
Some other considerations:
_
is considered exhaustive as it's "everything else", meaning there's an additional property - total match vs. exhaustiveness - to considerI'm in favor of this in theory, though I'd probably have to think about it more
Another con to this is that you can achieve this today by setting warnings as errors,
Actually, I'm not able to achieve this today, if matches use wildcards, adding a case will result in warnings (that are great) in places there are no wildcards.
The design of that optional restriction feels similar to [<RequiresQualifiedAccess>]
which also dictates how the matching is done.
Wouldn't be added to any types defined in FSharp.Core as this would break existing codeWouldn't be added to any types defined in FSharp.Core as this would break existing code
Agreed, this is really a feature to allow optional strictness on specific DUs, and I don't believe it applies to general libraries, but more to domain specific code; where having some extra explicitness on how matching should be done for selected entities seems like a win / great tool.
Cost in documentation + guidance, as _ is considered exhaustive as it's "everything else", meaning there's an additional property - total match vs. exhaustiveness - to consider
Agreed, there would probably be need to enrich few places in the documentation, language specification, guildelines, maybe most of it could remain concentrated on the attribute documentation, and would go along well with existing content touching on [<RequiresQualifiedAccess>]
.
Thanks for feedback guys!
What about nested pattern matching on these types? Consider these examples:
[<EnforceTotalMatch>]
type StrictDU = Enforced1 | Enforced2 | Enforced3
let strict = Enforced1
match Some strict with
| None -> "nothing"
| Some _ -> "something" // error?
match [ strict ] with
| [] -> "empty"
| _ -> "not empty" // error?
If the error happens for nested patterns, then I think it would be too impractical to use. If it doesn't happen on nested patterns, then the restriction quite easy to work around, limiting the usefulness of the feature. Maybe it would only actually be enforced if at least one case was explicitly mentioned?
@theprash great question!
I'm eager to see what other think about potential issues / inconsistencies that would make the feature useless or if it can be made sound / respecting the principle of least surprise for most F# users:
In both of the matches you shown, if the type of strict
is not resolved to a concrete DU type having the attribute, then it should compile fine (generic code).
If the compiler knows that strict
has for type a DU with the attribute, then it should fail in your sample, as you are using an explicit wildcard on the value in the first match.
The second match is ambiguous and it would definitely be more expensive for the compiler to figure out if a wildcard is OK or should fail than it is now; since the second match you are using is not binding the DU instance at all, I think that one should succeed the compilation.
then I think it would be too impractical to use
I agree that the second example is showing the kind of edge cases that would need to be nailed down, and more consistency/use cases considerations required for a RFC.
If it doesn't happen on nested patterns, then the restriction quite easy to work around
I don't think it should be let to happen; so long a _
or named/unrestricted binding is used as a binding for DU with this attribute, I'd expect a compile error.
I think this can be implemented consistently if, for the purpose of warnings or compilation, _
is not allowed to match DU types with [<EnforceTotalMatch>]
.
So in @theprash 's example,
match Some strict with
| None -> "nothing"
| Some _ -> "something" // Error: _ is not allowed to match a StrictDU
match [ strict ] with
| [] -> "empty"
| _ -> "not empty" // OK: _ is allowed to match a List<StrictDU> because List<_> does not have [<EnforceTotalMatch>]
In codebases that you control, I think advice to minimize use of _
is adequate and this suggestion is not needed. It could be useful when creating APIs that expose F# DUs for F# consumers.
I think there's a big difference between a _
that matches absolutely all cases, and a _
that matches cases not previously matched. ie between this:
match Some strict with
| None -> "nothing"
| Some _ -> "something" // matches everything
and this:
match Some strict with
| None -> "nothing"
| Some Enforced1 -> "something1"
| Some _ -> "something2" // matches remaining cases
To me, the first case is just a general ignore and should not cause an error. The second one is the case where an error would be useful.
Great discussion. Thinking about this at a bit more of a high level, there are two axes by which to grade a feature like this:
_
in certain contexts surprise you?I'm curious how folks here feel about @charlesroddie's example and @Tarmil's example on these two points.
Not sure why exhaustive match is not always preferred? This is the approach rust goes with, if match is used as an expression.
I dislike sprinkling the language with attributes here and there.
@Swoorup Exhaustive match is always preferred: F# gives a warning if a match isn't exhaustive. But this is specifically about failing when exhaustive match was achieved using a _
wildcard rather than explicitly listing all cases. That's why it only makes sense for some types but not others. Rust doesn't warn or fail about this either, nor should it by default:
enum MyEnum {
Case1,
Case2,
}
match myEnum {
Case1 => 1,
_ => 2, // No problem
}
@Swoorup
I dislike sprinkling the language with attributes here and there.
That implementation detail doesn't impact at all the consumption side, so it is a minor concern to me.
If you like the suggestion, what would you do to mark the DU you want it to apply on?
I get used to the name of attributes I find useful in my design, and I prefer that to keyword soups for this type of features. (it is also much easier to not have to change the grammar and carry in the type checker for a apprentice compiler maintainer)
@Tarmil: do you feel there is a tension between #222, the pre #222 defaults of F# and this suggestion?
I'm marking this as approved in principle. It's a great addition to allow enforcement of increased soundness of consuming code
The safety around exhaustive pattern matching is one of the strong point of F# type system,
For F# users, discarding some cases of a DU in a match is always a fine balance between future changes and state of the code using the DU.
I'm sure many F# users face situations while adding cases to the DU or reviewing / adjusting the existing matches or those to add asking self:
In specific cases, it would be relevant to make usage of the wildcard discouraged more than morally, but with the tools of software engineers: the compiler.
sample
What:
Why:
How To Fix:
Where:
NB: An error is produced opposed to the warning we usually get if there is no wildcard and missing cases.
Usage in other conditional constructs is an area of investigation / suggestions, maybe usage outside
match
andfunction
should lead to a new warning.This could be refined to apply to individual cases in a later revision, if the feature is relevant and doable.
Using this attribute could lead to a design time technique, where F# user would flip the attribute on while adding cases and removing it once all code changes and tests are done or to keep only in debug build; bringing some extra confidence of having reviewed all the matches.
Pros and Cons
The advantages of making this adjustment to F# are:
The disadvantages of making this adjustment to F# are:
_
is used (many places...)Extra information
Estimated cost (XS, S, M, L, XL, XXL): medium
rfc: small compiler: medium tests: medium-large documentation / guidelines: medium
Related suggestions: #414
Affidavit