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

Compiler allows non-functions to be bound to active pattern names #17190

Open
brianrourkeboll opened this issue May 20, 2024 · 4 comments
Open
Milestone

Comments

@brianrourkeboll
Copy link
Contributor

brianrourkeboll commented May 20, 2024

There are a couple of ways in which it is possible to define "active patterns" that are (from the F# perspective) values, not functions. While the tooling treats them as active patterns, it is impossible to actually use them as patterns in any pattern-matching position.

Expected behavior

The compiler disallows defining a multi-case active pattern that is not a function:

let (|P|Q|) = Choice<unit, unit>.Choice1Of2 ()
// error FS1209: Active pattern '|P|Q|' is not a function

I would expect the same for single-case and partial active patterns.

Actual behavior

(|P|)

A value bound to what looks like a single-case active pattern name is actually compiled as a property with a getter (when a module-bound value):

let (|P|) = 3
// val (|P|) : int = 3

image

(|P|_|)

An option or value option value bound to a partial active pattern name is compiled to the exact same .NET method as it would be if it were defined as a function, but from the F# perspective it is a value (or a type function, even though you cannot give it explicit type arguments), not a function, although the tooling again treats it as an active pattern.

let (|P|_|) = None
// val (|P|_|) : 'a option

image

let (|P|_|) = ValueNone
// val (|P|_|) : 'a voption

image

Unfortunately, updating the compiler to disallow these scenarios would be a breaking change — someone could be passing values so defined around qua values (f (|P|), let r = (|P|) + (|Q|), etc.) — although I doubt anyone actually intentionally binds values to active pattern names like this in normal code.

@github-actions github-actions bot added this to the Backlog milestone May 20, 2024
@edgarfgp
Copy link
Contributor

A warning behind a lang feature like we have done in other cases might work here ?

@brianrourkeboll
Copy link
Contributor Author

brianrourkeboll commented May 20, 2024

A warning behind a lang feature like we have done in other cases might work here ?

Yeah, I think that would probably be the best we could do.

@brianrourkeboll
Copy link
Contributor Author

brianrourkeboll commented May 20, 2024

It turns out that members with active pattern names are also allowed (SharpLab) and are not treated as patterns at all by the tooling:

[<AutoOpen>]
type Foo =
    static member (|P|) = 3
    static member (|Q|_|) = 3
    static member (|R|_|) = None
    static member (|S|_|) = ValueNone
    static member (|T|U|) = 3

image

Anonymous records, too:

let x = {| (|P|) = 3 |}

(Parens are gone in tooling, though)

image

And constraint calls:

let inline f x = (^a : (member (|P|) : int) x)
f {| (|P|) = 3 |}

And lambdas:

let f : int -> int = fun (|P|) -> (|P|) + (|P|)

Or any other pattern-matching context:

match 3 with
| (|P|) -> (|P|) + (|P|)

Also unions:

[<RequireQualifiedAccess>]
type T =
    | (|P|) of int
    | (|Q|)

I guess it's a similar phenomenon to the one that allows you to bind operator names to values:

let y = {| (||) = 3 |}
match (+) with (|++|) -> 2 |++| 2
let f (>*^*<) x y = x >*^*< y
match (+) 3 with (~+) & (~-) -> +(-(2)) + -(9) // 20.

They're all just being lexed as IDENTs (or parenIdents), and then there's inconsistent treatment later on...

@abelbraaksma
Copy link
Contributor

Yes. from F#'s standpoint, these are just longident, and while they have special meaning in some scenarios, they are not illegal. Likewise, if you do have an active pattern, you can just use it as a HOF function argument, or in a function call.

I vaguely remember bringing this up ages ago, but I cannot remember the resolution at the time. We could (should?) issue a warning perhaps, instead of an error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New
Development

No branches or pull requests

3 participants