fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
344 stars 21 forks source link

"Incomplete pattern matches" warning with when guards on clearly complete patterns #1371

Open Thorium opened 5 months ago

Thorium commented 5 months ago

I propose that the compiler would check the when-guard conditions on basic Boolean algebra level so that there wouldn't be warning of FS0025 "Incomplete pattern matches on this expression" if there is clearly logically no incomplete pattern.

The existing way of F# is to raise warning, for example: image

The current workaround is to add there a "never"-option, but this also means the check is done on run-time and not on compile-time.

let f x =
    match x with
    | a when a >= 0 -> 1
    | a when a < 0 -> 2
    | never -> failwith "should never happen"

I could just leave the last guard away, but for the the source-code readability it would be nice to be able to have it. So that the next developer understands what is the last "else" condition all about.

I get it that sometimes it's impossible to tell, but if you have when-conditions with the complete space like

Extra information

Estimated cost (XS, S, M, L, XL, XXL): Sorry, I have no idea...

Related suggestions: This is so obvious issue I thought there is already suggestion, but I just couldn't find any.

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

Please tick all that apply:

For Readers

If you would like to see this issue implemented, please click the :+1: emoji on this issue. These counts are used to generally order the suggestions by engagement.

brianrourkeboll commented 5 months ago

What if someone shadowed not or the equality, comparison, or Boolean operators? What if someone defined misbehaving equality or comparison implementations on their own types? What if myBool was a mutable value that got mutated on another thread between the first and second when?

I think that such considerations would probably mean that this would be limited to types and operators well-known to the compiler — maybe even just primitives, since otherwise any nested value in an object graph participating in equality or comparison could invalidate the guarantee.

vzarytovskii commented 5 months ago

Basically as Brian mentioned - we can't really guarantee how does "when" part behave at runtime vs compile time.

Thorium commented 5 months ago

How does the normal match (without when guards) goes around this issue? What if you override op-equality and GetHashcode with random variables?

I guess my issue is rather design: if it would be clear that you must write the default case last... But my thought process eagerly thinks the most interesting and probable case first. Then it's bit un-intuitive to remove guards and add other cases from bottom-up direction:

match myVal with
| x when x >= minValue && x <= maxValue -> // oh no this is the base-case and should be last
T-Gro commented 5 months ago

I could imagine a simplified version of this for a condExpression and not(condExpression), when using the FSharp.Core's not function. But if this is the case, a nested match will do just as well.

As soon as operators, arithmetic's or runtime calls are involved ( = the practical cases), I agree with @brianrourkeboll comment above.

I would also add to the list of workarounds that active patterns can help with the stated problem and still maintain totality of compiler checking:

let inline (|PositiveOrZero|Negative|) x = if x >= 0 then PositiveOrZero else Negative

let f x =
    match x with
    | PositiveOrZero -> 1
    | Negative -> 2
charlesroddie commented 2 months ago

This often gets asked in forums. It's an understandable request since the compiler should be able to figure things out at least in the case of bools. (Inequality requires knowing about whether the type is totally ordered w.r.t. inequality operators).

However, most of the requests come from an excessive use of match, similarly to the example above. Curiously, they also typically come with an additional unnecessary variable binding, also as in the example above.

The fix is to use if, and 95% of the time the resulting code will be tidier. The remaining 5% being deeply nested matches that might not end up tidier. If this issue is about those cases, then seeing an example would be good.

Thorium commented 2 months ago

The point of original code was not to be the most efficient, but rather most explicit in business and maintainability sense, so that there are no else-branch-leaks in the flow.

I don't see how this code is much better, it still needs else-case that it won't compile without:

if a >= 0 then 1
elif a < 0 then 2
else failwith "should never happen"

If you leave else away, the compiler will claim that int and Unit are not compatible.

voronoipotato commented 2 days ago

What if someone shadowed not or the equality, comparison, or Boolean operators? What if someone defined misbehaving equality or comparison implementations on their own types? What if myBool was a mutable value that got mutated on another thread between the first and second when?

I think that such considerations would probably mean that this would be limited to types and operators well-known to the compiler — maybe even just primitives, since otherwise any nested value in an object graph participating in equality or comparison could invalidate the guarantee.

I will say, I think if someone changes the operation to mean something other than the meaning of the operation, I think showing a warning there when it doesn't apply is perfectly reasonable.