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

Special inline active patterns that have an effect on exhaustiveness warnings #755

Open
Happypig375 opened this issue Jun 14, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@Happypig375
Copy link
Contributor

commented Jun 14, 2019

Special inline active patterns that have an effect on exhaustiveness warnings

I propose we let simple inline active patterns have an effect on exhaustiveness, through an [<ExhaustionAnalysis>] attribute.

Consider:

type DU = DU1 | DU2 | DU3
let inline (|DU1or2|_|) x = match x with DU1 | DU2 -> Some DU1or2 | _ -> None
let du = match System.Random().Next 3 with 0 -> DU1 | 1 -> DU2 | _ -> DU3
match du with
| DU1or2 -> printfn "1 or 2"
| DU3 -> printfn "3"
  match du with
  ------^^
warning FS0025: Incomplete pattern matches on this expression. For example, the value 'DU1' may indicate a case not covered by the pattern(s).

Compare with:

type DU = DU1 | DU2 | DU3
[<ExhaustionAnalysis>]
let inline (|DU1or2|_|) x = match x with DU1 | DU2 -> Some DU1or2 | _ -> None
let du = match System.Random().Next 3 with 0 -> DU1 | 1 -> DU2 | _ -> DU3
match du with
| DU1or2 -> printfn "1 or 2"
| DU3 -> printfn "3"
// No warnings

The existing way of approaching this problem in F# is copying the active pattern everywhere. It's hard to update them.

What is allowed?

In line with the current exhaustion analysis, aka

  • Function body only contains a match
  • No when
  • The match doesn't call other non-inlined functions
  • No mutation
  • No non-match expressions
  • Patterns only

So, these will not be considered:

[<ExhaustionAnalysis>]
let inline (|Nope|_|) x = match x with DU1 | DU2 -> if ExternalBoolean then Some Nope else None | _ -> None
//error at if
[<ExhaustionAnalysis>]
let inline (|Nope2|_|) x = match x with DU1 | DU2 when false -> Some Nope2 | _ -> None
//error at when
[<ExhaustionAnalysis>]
let inline (|Nope3|_|) x = match x with DU1 | DU2 -> printf "Hi"; Some Nope3 | _ -> None
//error at printf
[<ExhaustionAnalysis>]
let (|Nope4|_|) x = match x with DU1 | DU2 -> Some Nope4 | _ -> None
//error at let, inline must be declared

and all other stuff that cause exhaustion analysis to be indeterminate.
This will be fine:

[<ExhaustionAnalysis>]
let inline (|DU1or2|_|) x = match x with DU1 | DU2 -> Some DU1or2 | _ -> None

Pros and Cons

The advantages of making this adjustment to F# are

  1. More uses of exhaustion analysis along with active patterns
  2. No need for unnecessary exception-raising branches
  3. Increased quality of life of developers - the compiler does the heavy lifting

The disadvantage of making this adjustment to F# is the huge amount of effort this would probably require.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): L ~ XL

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick this 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
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • 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.

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
@smoothdeveloper

This comment has been minimized.

Copy link

commented Jun 14, 2019

Do we actually need a specific attribute or this could be added without breaking changes?

Existing warning on currently exhaustive checks (similar to sample) would disappear, is there anything else?

Somehow related #731

@Happypig375

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

#731 is not really related. #731 modifies the DU itself, this modifies active patterns. However, both influence exhaustive checks.

@Happypig375

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

For instance, this can be possible as well:

[<ExhaustionAnalysis>]
let inline (|RecordContainsTrue|_|) x = match x with { X = true } -> Some RecordContainsTrue | _ -> None
match { X = SomeBoolean } with
| RecordContainsTrue -> printfn "true"
| { X = false } -> printfn "false"
//No warnings.
@smoothdeveloper

This comment has been minimized.

Copy link

commented Jun 14, 2019

@Happypig375 what would be the cons of having this work as is without the attribute (assuming we are fine with incorrect exhaustivity warnings currently showing eventually going away)?

@Happypig375

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2019

Unpredictability - how would you be sure that your active pattern does affect exhaustivity? Surely the compiler cannot tell if all active patterns are pure. Some might call impure functions and suddenly unexhausted match warnings pop up everywhere.

That is the single reason that the attribute should be required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.