Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
4 of 6 tasks
Thorium opened this issue Jun 14, 2024 · 7 comments
Open
4 of 6 tasks

Comments

@Thorium
Copy link

Thorium commented Jun 14, 2024

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

  • when (myBool) and when (not myBool)
  • when y <= x and when y > x

...then it's totally possible to know.

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:

  • This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
  • This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

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

@brianrourkeboll
Copy link

brianrourkeboll commented Jun 14, 2024

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
Copy link

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

@Thorium
Copy link
Author

Thorium commented Jun 14, 2024

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
Copy link

T-Gro commented Jun 17, 2024

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
Copy link

charlesroddie commented Sep 7, 2024

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
Copy link
Author

Thorium commented Sep 8, 2024

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants