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
proposal: enable warning 6 ([labels-omitted]
) by default
#10118
Comments
(At some point we should probably review the warnings enabled by default in the compiler, and with Dune, and see if we could/should try to remove some of the differences. It's not so good to have a large discrepancy between observed warnings across the two systems.) |
I'm not an expert on this topic but I'm broadly in favor of having this warning "on" by default, and perhaps move towards stricter handling of labels in the future. If I remember correctly (otherwise @garrigue will correct me), when labeled arguments from OLabl were added to OCaml, we first hoped that labels at application sites could be kept entirely optional, so that the stdlib function could be labeled without breaking existing code. This turned out to be possible for full applications (hence the support we have today) but not for partial applications (hence the two versions labeled/unlabeled for stdlib modules). Nowadays, I think that labels on a function definition should be honored at all call sites. It is clearer and simpler this way. |
I think that would be an excellent idea, here's a data point. |
It's not so easy to get feedback on this proposal, so I think that I will turn this issue into a PR. Somehow people react more consistently when we are one click away from the thing they don't like. |
#10140 is the PR with the change. I propose to migrate further discussion there. Thanks! |
OCaml understands full unlabelled application of labelled functions, but emits warning 6 (
[labels-omitted]
). This warning is not enabled by default -- but it is part of the Dune default warnings, so in practice it is the default for a large part of the userbase today.I believe that the reason for not enabling it by default comes from a "transitory" period when labelled functions were introduced. I don't see a good reason today to keep encouraging this programming style.
On the other hand, when an API designer uses label, they generally consider that their users will have to use them; it is used to clarify caller-side code, and in particular to avoid the programming error of mistakenly swapping arguments of compatible type. Encouraging non-labelled applications weakens these benefits.
This suggestion originates from #10105, which considers replacing a non-labelled function with a labelled function using a different argument order. Currently (with warning 6 disabled by default), non-labelled callsites will keep compiling just fine, even if they forgot to fix the argument order, leading to silent bugs introduced by innocuous refactorings.
The text was updated successfully, but these errors were encountered: