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

Defaults to optional arguments suppress warning 68 in hard-to-predict ways #12175

Closed
ncik-roberts opened this issue Apr 10, 2023 · 0 comments · Fixed by #12236
Closed

Defaults to optional arguments suppress warning 68 in hard-to-predict ways #12175

ncik-roberts opened this issue Apr 10, 2023 · 0 comments · Fixed by #12236
Assignees

Comments

@ncik-roberts
Copy link
Contributor

Note

This issue will be fixed by an implementation for ocaml/RFCs#32, which I'm working on. That RFC fixes this issue by specifying the evaluation order of optional argument defaults / pattern matches on mutable state, which removes the need for warning 68. In the meantime, I'm writing this to log and track a perhaps-surprising behavior.


Bug

Warning 68 notes when matching on mutable state (e.g. a mutable record field) prevents a function from being uncurried:

# let f1 { contents } x = contents + x;;
Warning 68 [match-on-mutable-state-prevent-uncurry]: This pattern depends on mutable state.
It prevents the remaining arguments from being uncurried, which will cause additional closure allocations.
val f1 : int ref -> int -> int = <fun>

But placing an optional argument with a default value immediately before the pattern suppresses that warning:

# let f2 ?(foo = print_endline "evaluated") { contents } x = contents + x;;
val f2 : ?foo:unit -> int ref -> int -> int = <fun>
# f2 (ref 0);;
evaluated
- : int -> int = <fun>

This behavior is arguably ok if you think of the optional argument default, and not the mutable field pattern, as being the reason that uncurrying is interrupted. (So, perhaps it's confusing to mention the mutable field pattern in the warning.) But, the current behavior could be undesirable for a few reasons:

  • The warning raises in an unpredictable set of circumstances
  • The warning doesn't even catch all cases where removing a pattern match on a mutable field would allow the uncurrying translation to kick in. For example, if we remove the mutable field pattern from the above example, the uncurrying still happens such that the evaluation of the default argument is delayed until the function is fully applied:
# let f3 ?(foo = print_endline "evaluated") x y = !x + y;;;
val f3 : ?foo:unit -> int ref -> int -> int = <fun>
# f3 (ref 1);;
# f3 (ref 1) 2;;
evaluated
- : int = 3
@gasche gasche self-assigned this Jun 28, 2023
@gasche gasche linked a pull request Jun 28, 2023 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants