[WIP][Typing/typecore.ml]: Added warning 68, matching a function type #9395
Conversation
| @@ -775,6 +779,8 @@ let descriptions = | |||
| 64, "-unsafe used with a preprocessor returning a syntax tree"; | |||
| 65, "Type declaration defining a new '()' constructor"; | |||
| 66, "Unused open! statement"; | |||
| (* 67, "ask octachron" *) | |||
There was a problem hiding this comment.
Indeed, the short description for the unused functor argument warning is missing. If you want to add the missing text, it would make for a fine separate PR. Otherwise, myself (or @trefis) will fix the issue.
There was a problem hiding this comment.
Okay I ll add it.
|
Concerning the failing tests, you can deactivate the warning locally with |
| exists_general_pattern { f = fun (type k) (p : k general_pattern) -> | ||
| match p.pat_desc with | ||
| | Tpat_exception _ -> true | ||
| | _ -> false } pat |
There was a problem hiding this comment.
This is a quite self-contained function that is used in two different places. It is often better in this situation to make the function a toplevel helper function shared between the two call sites.
|
Sorry to be the grumpy one, but I think this PR is premature. I don't think there's a consensus around the idea behind this PR, i.e. I don't think the silence that followed @damiendoligez's proposal was an approving one. I am personally not convinced by the approach, matching on a function type is not necessarily a mistake. The issue lies with the ignoring of the result, not with the match itself. And while we do not have a satisfying answer to that issue yet, I'm not keen on adding "workaround" warnings. |
|
You are perfectly right to express your disagreement, and thanks for stating it early. Do you have in mind a precise example in which matching on a syntactic value that is function (or any other type that cannot be destructured) is not a mistake? |
|
Matching on a partial application of a function to catch an exception in the construction of a cloture might be a reasonable use I'd guess ? |
|
I agree, this is why I added the "syntactic value" clause. And the current code only emits a warning if there are not exception pattern in the cases of the match. |
|
I am closing this PR as non-consensual, partially subsumed by #10328 and not being currently active. |
#6881
@Octachron
# match fun x -> x with x -> x, x;; Warning 68: Trying to match a function type. - : ('a -> 'a) * ('b -> 'b) = (, )I will improve the code structure and add the Changes files once the PR reaches to its working state.
Thanks a lot for all the help