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
Controlling warnings locally #6202
Comments
Comment author: @hcarty I think this would be very useful for the reasons you mentioned. It could also be useful for open!-like use of the M.(...) syntax. |
Comment author: @garrigue I think this is a good idea, but we must be very careful about readability and clarity of the semantics. |
Comment author: @alainfrisch Commit 14214 on trunk: the 'warning' attribute is interpreted on expressions (the scope is restricted to the target expression) and in floating signature/structure attributes (with a global scope, not even restricted to the current signature or structure). There is a subtle interaction between the two: when an expression has a 'warning' attribute, the state of warnings before the expression is type-checked is restored after the expression has been type-checked, which cancels the effect of floating attributes within the expression (in a local module expression under that expression). Notes:
Comments are welcome! |
Comment author: @lpw25 As a side note, we should start putting a list of these "reserved" attributes somewhere so that people know not to use them for their own extensions. |
Comment author: @zoggy A warning could be added to indicate that some warnings were turned off/on locally, so that using this new warning can be used on the command line to remove all locally turned on/off warnings. +1 on having the possibility to have attributes before items they refer to, not only after. |
Comment author: @alainfrisch
Do you think we should use a special namespace convention for attributes which are given a special meaning by the OCaml compiler itself? E.g. [@ocaml.deprecated], [@ocaml.warning] ? |
Comment author: @damiendoligez +1000 on Maxence's idea of a new warning that gets triggered every time this feature is used. |
Comment author: @hcarty A special or reserved attribute prefix for attributes provided by the OCaml compiler is a useful idea. These attributes don't require an extra ppx - it's nice to immediately know which attributes can be used without external tools. |
Comment author: @alainfrisch Attribute renamed to ocaml.warning. The syntax of floating attributes as changed as well: [@@@ocaml.warning "..."] |
Comment author: @alainfrisch I'm a little bit unsatisfied with local control of warnings. While it is well-defined to control warnings for a compilation unit, it is less clear what the exact meaning is for even more local control. Concretely, many warnings are issued by the type-checker in a final pass (e.g. unused stuff warnings), not while type-checking an inner component. Currently [@@@ocaml.warning] changes the set of active warnings while type-checking such inner components, so this does not interact nicely. A better defined behavior would be to filter warnings according to their location, independently of where in the type-checker code they are issued. This is a bit more complex to implement, though. Typically, functions such as Location.prerr_warning should take an extra Warnings.state parameter. Alternatively, it could make sense to avoid keeping warning configuration in a global state and store it in the Env.t instead (which would force to think about the relevant scope when issuing a warning or checking that one warning is enabled). edit: there is already some provision so that when a delayed check is run, the warning configuration from the point it was registered is restored. |
Comment author: @damiendoligez This feature interferes badly with the OCAMLPARAM feature. We need to give OCAMLPARAM precedence over local changes. |
Comment author: @alainfrisch Keeping this opened. The implementation needs to be improved and the interaction with OCAMLPARAM discussed. Let's consider the existing feature as experimental. |
Comment author: @alainfrisch I'm not sure that OCAMLPARAM should take precedence over local controls. |
Comment author: @alainfrisch The feature works much better in 4.06. I'm thus closing this issue. Damien: you mentioned that OCAMLPARAM should take precedence. But I see OCAMLPARAM as a way to inject flags at the level of the command-line (without having to patch build systems), and with this interpretation it should not take precedence over attributes. Feel free to open another ticket if you feel strongly about your point of view and would like us to discuss further. |
Original bug ID: 6202
Reporter: @alainfrisch
Assigned to: @alainfrisch
Status: resolved (set by @alainfrisch on 2017-10-09T13:58:15Z)
Resolution: fixed
Priority: normal
Severity: feature
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: misc
Related to: #6483
Monitored by: @gasche @ygrek @hcarty
Bug description
It would be useful to specify the warning behavior (which ones are enabled, which ones are turned into errors) locally, within the source code itself, and even better, to have local control within a single unit.
For instance, it makes sense to disable some "unused stuff" warnings locally on a piece of code under development. Or locally disable warning 40 (label/constructor used out of scope) in a specific piece of code which makes heavy use of this feature (while not allowing it for the whole project or unit).
What about using attributes to control this behavior? One could either rely on "floating item attributes":
;;[@@warning "-32..39" ]
and/or interpret attributes on expressions/module expression/etc:
let x =
begin[@warning "-32..39"]
....
end
module X = struct end [@warning "-32..39"]
Opinions?
The text was updated successfully, but these errors were encountered: