Add mnemonics for warnings

[nojb]

This PR proposes to add easier-to-remember names for warnings as an alternative to numeric IDs.

I derived the names mechanically from the warning constructor name; for simplicity I would suggest this always be the case, so that if a name needs to be tweaked, then the constructor name should be changed accordingly. (NB: what follows is the description of the final design, the original description can be found further below).

For the CLI, the -w flag is extended to take a single name in the form:

-w +<name> (or: -w <name>)
-w -<name>
-w @<name>

respectively to enable, disable, or enable as fatal error the given named warning. For example,

ocamlc -w unused-value-declaration -w -unused-open foo.ml

enables unused-value-declaration (warning 32) and disables unused-open (warning 33). The names for each warning are can be found in the output of -warn-help, in the manual, and when a warning is displayed by the compiler, eg:

Warning 8 [partial-match]: this pattern-matching is not exhaustive.
All clauses in this pattern-matching are guarded.

or, in the case of fatal warnings:

Error (warning 8 [partial-match]): this pattern-matching is not exhaustive.
All clauses in this pattern-matching are guarded.

Using a name to specify a single warning can be used anywhere where a warning specification list is expected, eg:

[@@@warning "-unused-value-declaration"]

Old description (so that the discussion below makes sense)

For the CLI, I used two new flags, -W <name> (to enable a warning by name) and -Wno <name> (to disable a warning by name). For example,

ocamlc -W unused-value-declaration -Wno unused-open foo.ml

enables unused-value-declaration (warning 32) and disables unused-open (warning 33). The names for each warning are displayed when using -warn-help.

Similarly, the flags -Werror <name> and -Wno-error <name> can be used to toggle warning <name> into an error.

Next step is to allow its use when using the [@@ocaml.warning ...] attribute, but wanted to put this out there in order to make sure there is agreement on the approach.

Another possible follow-up is to introduce names for sets of warnings, like -W all (similar to -w A), etc.

Closes: #7823

[gasche]

Thanks! I would love to have real names for warnings, especially in local attributes. I can cope with arcane build scripts, but it feels really bad to commit [@@warning "-4"] in a source file.

High-level remarks:

1. I like your systematic approach to naming, but I noticed a fair amount of typos in the actual strings. We have to be careful here. In particular, if a release goes out with the bad name, it could be fairly annoying for compatibility.

2. I was surprised to see that you introduced new flags. I am fine with the new flags, but maybe there could be also an approach for people using -w as before? For example, maybe we could single-quote or parenthesize warning names in this context. (I suppose that not using any form of quoting is problematic as - is ambiguous, but then a longest-match rule would also deal with this just fine.)

3. I have long thought about turning warnings into a sigma-type, with one GADT for the warning name (indexed over the payload type), so that we could manipulate warning names in a more convenient way. Would you be interested in a preparatory PR doing this?

[gasche]

typo here

[nojb]

Thanks, fixed.

[gasche]

the two names are different here

[nojb]

Thanks, fixed.

[gasche]

Typo here

[nojb]

Thanks, fixed.

[gasche]

This should probably be renamed into definitions (but in fact the name could be changed completely, maybe Module_name_conflict or something; for another PR).

[nojb]

I agree. I hold off to changing this one until the general discussion settles down to avoid polluting the diff.

[gasche]

arguments

[nojb]

Thanks, fixed.

[nojb]

1. I like your systematic approach to naming, but I noticed a fair amount of typos in the actual strings. We have to be careful here. In particular, if a release goes out with the bad name, it could be fairly annoying for compatibility.

Indeed. I fixed the ones you pointed out. We could also imagine putting the warning definitions in their own file and generating the names with a script. Is it worth it?

2. I was surprised to see that you introduced new flags. I am fine with the new flags, but maybe there could be also an approach for people using -w as before? For example, maybe we could single-quote or parenthesize warning names in this context. (I suppose that not using any form of quoting is problematic as - is ambiguous, but then a longest-match rule would also deal with this just fine.)

Using names with -w seemed a big kludgy. In a sense my view is that the format used by -w is well-suited to enable/disable ranges of numeric IDs, while names are more suited to enabling/disabling individual warnings. Having said that, if there is agreement on a specific format, why not? I kind of warmed up to the idea of having the separate flags, though.

3. I have long thought about turning warnings into a sigma-type, with one GADT for the warning name (indexed over the payload type), so that we could manipulate warning names in a more convenient way. Would you be interested in a preparatory PR doing this?

I thought about this briefly. If I understand correctly, what we would gain is not having to use dummy values to instantiate warnings, like it is done for Warnings.all in this PR or in the typechecker to check if a given warning is active or not. At first sight, it does not seem like much, but sure, why not?

[nojb]

One important point that I forgot in the issue description: the interaction with alerts. In my view we could use the same flags (-W, -Wno, -Werror, -Wno-error) to also control the alert settings (with warnings taking precedence in case of clashes). In my experience the existing flag for alerts -alert <mod><name> with <mod> = +, -, ++, --, @ is not very intuitive.

[nojb]

1. I was surprised to see that you introduced new flags. I am fine with the new flags, but maybe there could be also an approach for people using -w as before? For example, maybe we could single-quote or parenthesize warning names in this context. (I suppose that not using any form of quoting is problematic as - is ambiguous, but then a longest-match rule would also deal with this just fine.)

Using names with -w seemed a big kludgy. In a sense my view is that the format used by -w is well-suited to enable/disable ranges of numeric IDs, while names are more suited to enabling/disabling individual warnings. Having said that, if there is agreement on a specific format, why not? I kind of warmed up to the idea of having the separate flags, though.

After sleeping on it, I changed tack and got rid of the new flags, and simply adapted the existing flag -w to accept names, but only one at a time. This way we don't have to worry about escaping -. Concretely, -w now accepts: -w +<name>, -w -<name> and -w @<name> (meaning obvious). -w <name> is a synonym for -w +<name>.

[nojb]

BTW, a consequence of the above is that the following now works:

[@@@warning "-unused-value-declaration"]

[gasche]

Thanks! I didn't dislike your new flags (they are arguably a better interface that could be proposed independently), but I like the idea that we can extend the previous flag with the new feature, and the side-effect that attributes work is particularly nice.

[gasche]

I have reviewed the code and I believe it is correct (modulo the bug, see comments). One thing that is remarkable is how non-invasive the patch is; it's really simple.

I am morally approving this PR, because I want to have the feature, but I'm not formally Approving because this deserves a broader discussion with other maintainers¹ and I don't want us to get tempted into merging before.

¹: it's not obvious, but when I have mentioned this feature to other maintainers in the past, I got the feeling that they considered it as a minefield that deserved careful discussion.

[gasche]

I believe that this is subtly wrong if we have warnings named foo and also oo. You should check s before rest.

(Nitpick: I would match on s.[0], name_to_number rest rather than in the other order, so that source order follows input order.)

[nojb]

Good catch! Fixed.

[gasche]

I think that it would be better to use the Objectively Right indentation style here, which is to use a terminator for the last item and the enclosing bracket on the next line, because we are going to keep adding new stuff to this list.

(What happens when we forget ? Could there maybe be a check somewhere that the last warning number is also the last known warning?)

[nojb]

Fixed indentation. To make sure that we are not forgetting anything, one possibility could generate the list all and the function name with a script.

[nojb]

I am morally approving this PR, because I want to have the feature, but I'm not formally Approving because this deserves a broader discussion with other maintainers¹ and I don't want us to get tempted into merging before.

Thanks!

I also hope other developers will chime in. Do you think it would be too intrusive to cc the ocaml-dev team in this kind of PRs that need wide discussion?

[gasche]

Let's try! Yo @ocaml/caml-devel, do you have an opinion about adding names for warnings (systematically derived from the constructor name in Warnings.ml)? This is proposed by @nojb, and the implementation is surprisingly simple.

-w +unused-value-declaration
-w -unused-value-declaration
-w @unused-value-declaration
[@@@warning "-unused-value-declaration"]

[Armael]

I think it's a great idea, and I like the UI.
Regarding the implementation, the use of all and dummy payloads makes me a bit uncomfortable. Wasn't it possible to simply extend the description list and have it contains triples (warning number, warning name, warning description)? (then you can still have your hashtable on the side if you want)

[gasche]

@nojb: I foresee changes in warning names in the future, and then we will really want to also keep supporting the older names for compatibility. The current implementation makes it hard to map several names to the same warning number. Do you have an idea?

[nojb]

Regarding the implementation, the use of all and dummy payloads makes me a bit uncomfortable. Wasn't it possible to simply extend the description list and have it contains triples (warning number, warning name, warning description)? (then you can still have your hashtable on the side if you want)

Yes, it was possible. I switched to using functions to try to exploit exhaustivity of pattern matching to make sure we have defined description, names, etc. for every warning. But of course I just pushed the problem over to the definition of all.

No problem from me to switch back to keeping a list of triples (warning number, warning name, warning description). In fact we could have a list of warning names to handle the case of deprecated names alluded to by @gasche.

@Armael @gasche do you agree to switch to a list of triples (warning number, warning names, warning description) : int * string list * string? If yes, I will update the PR.

[gasche]

Yes, I'm also happy with this proposed approach.

[nojb]

@Armael @gasche do you agree to switch to a list of triples (warning number, warning names, warning description) : int * string list * string? If yes, I will update the PR.

I pushed a commit with this change. One downside of it is that we lose the proximity in the program text between the warning name and the constructor, so it becomes harder to check that the warning name corresponds to the constructor (keeping this proximity was one of the reasons for my initial approach in fact).

[lpw25]

My only issue is that the current constructor names (assuming you haven't changed them) are not good names. For example, I bet anyone writing [@warning "+unused-argument"] really wants [@warning "+unused_strict_var"]. I think that a systematic assessment of all the names is needed before they become part of the public interface.

[fpottier]

I think that a systematic assessment of all the names is needed before they become part of the public interface.

This sounds reasonable indeed!

[nojb]

My only issue is that the current constructor names (assuming you haven't changed them) are not good names. For example, I bet anyone writing [@warning "+unused-argument"] really wants [@warning "+unused_strict_var"]. I think that a systematic assessment of all the names is needed before they become part of the public interface.

No, I haven't changed them. I will happily do the adaptation of the code if someone is willing to the assessment (I don't feel like I would do a very good job at it myself).

[shindere]

Nicolás Ojeda Bär (2020/06/11 22:04 -0700):

> @Armael @gasche do you agree to switch to a list of triples `(warning number, warning names, warning description) : int * string list * string`? If yes, I will update the PR. I pushed a commit with this change. One downside of it is that we lose the proximity in the program text between the warning name and the constructor, so it becomes harder to check that the warning name corresponds to the constructor (keeping this proximity was one of the reasons for my initial approach in fact).

I'm wondering about deprecation. Among the warning names, do we want to have a way to distinguish the deprecated ones, which might trigger a deprecation message, from the other ones? If each warning has only one official name, then the convention could be that this official name is the head of the list and that the deprecated equivalents are in the tail of the list.

[nojb]

This sounds reasonable indeed!

To help the task, I generated this table with the current names. Those interested may edit this issue directly (please do not delete the previous constructor name, but rather use overwrite instead).

Num	Constructor	Description
1	Comment_start	Suspicious-looking start-of-comment mark.
2	Comment_not_end	Suspicious-looking end-of-comment mark.
3	Deprecated	Deprecated synonym for the 'deprecated' alert.
4	Fragile_match	Fragile pattern matching: matching that will remain complete even if additional constructors are added to one of the variant types matched.
5	Partial_application	Partially applied function: expression whose result has function type and is ignored.
6	Labels_omitted	Label omitted in function application.
7	Method_override	Method overridden.
8	Partial_match	Partial match: missing cases in pattern-matching.
9	Non_closed_record_pattern	Missing fields in a record pattern.
10	Statement_type Non_unit_statement	Expression on the left-hand side of a sequence that doesn't have type "unit" (and that is not a function, see warning number 5).
11	Unused_match Unused_case	Redundant case in a pattern matching (unused match case).
12	Unused_pat Unused_subpat	Redundant sub-pattern in a pattern-matching.
13	Instance_variable_override	Instance variable overridden.
14	Illegal_backslash	Illegal backslash escape in a string constant.
15	Implicit_public_methods	Private method made public implicitly.
16	Unerasable_optional_argument	Unerasable optional argument.
17	Undeclared_virtual_method	Undeclared virtual method.
18	Not_principal	Non-principal type.
19	Without_principality Not_principal_labels	Type without principality.
20	Unused_argument Unreachable_argument	Unused function argument.
21	Nonreturning_statement	Non-returning statement.
22	Preprocessor	Preprocessor warning.
23	Useless_record_with	Useless record "with" clause.
24	Bad_module_name	Bad module name: the source file name is not a valid OCaml module name.
25	All_clauses_guarded	Deprecated: now part of warning 8.
26	Unused_var	Suspicious unused variable: unused variable that is bound with "let" or "as", and doesn't start with an underscore ("_") character.
27	Unused_var_strict	Innocuous unused variable: unused variable that is not bound with "let" nor "as", and doesn't start with an underscore ("_") character.
28	Wildcard_arg_to_constant_constr	Wildcard pattern given as argument to a constant constructor.
29	Eol_in_string	Unescaped end-of-line in a string constant (non-portable code).
30	Duplicate_definitions	Two labels or constructors of the same name are defined in two mutually recursive types.
31	Multiple_definition Module_linked_twice	A module is linked twice in the same executable.
32	Unused_value_declaration	Unused value declaration.
33	Unused_open	Unused open statement.
34	Unused_type_declaration	Unused type declaration.
35	Unused_for_index	Unused for-loop index.
36	Unused_ancestor	Unused ancestor variable.
37	Unused_constructor	Unused constructor.
38	Unused_extension	Unused extension constructor.
39	Unused_rec_flag	Unused rec flag.
40	Name_out_of_scope	Constructor or label name used out of scope.
41	Ambiguous_name	Ambiguous constructor or label name.
42	Disambiguated_name	Disambiguated constructor or label name (compatibility warning).
43	Nonoptional_label	Nonoptional label applied as optional.
44	Open_shadow_identifier	Open statement shadows an already defined identifier.
45	Open_shadow_label_constructor	Open statement shadows an already defined label or constructor.
46	Bad_env_variable	Error in environment variable.
47	Attribute_payload	Illegal attribute payload.
48	Eliminated_optional_arguments	Implicit elimination of optional arguments.
49	No_cmi_file	Absent cmi file when looking up module alias.
50	Bad_docstring Unexpected_docstring	Unexpected documentation comment.
51	Expect_tailcall Tailcall_expected	Warning on non-tail calls if @tailcall present.
52	Fragile_literal_pattern	Fragile constant pattern.
53	Misplaced_attribute	Attribute cannot appear in this context.
54	Duplicated_attribute	Attribute used more than once on an expression.
55	Inlining_impossible	Inlining impossible.
56	Unreachable_case	Unreachable case in a pattern-matching (based on type information).
57	Ambiguous_pattern Ambiguous_var_in_pattern_guard?	Ambiguous or-pattern variables under guard.
58	No_cmx_file	Missing cmx file.
59	Assignment_to_non_mutable_value Flambda_assignment_to_non_mutable_value	Assignment to non-mutable value.
60	Unused_module	Unused module declaration.
61	Unboxable_type_in_prim_decl	Unboxable type in primitive declaration.
62	Constraint_on_gadt	Type constraint on GADT type declaration.
63	Erroneous_printed_signature	Erroneous printed signature.
64	Unsafe_without_parsing No_unsafe_array_syntax_without_parsing	-unsafe used with a preprocessor returning a syntax tree.
65	Redefining_unit	Type declaration defining a new '()' constructor.
66	Unused_open_bang	Unused open! statement.
67	Unused_functor_parameter	Unused functor parameter.

[gasche]

Note: with a list of names for each warning, we have the option to not offer names for some warnings if we don't agree on a good name. So we could keep @nojb's implementation work, but start with only a handful of names that people like, and keep numbers-only for some warnings.

(I'm not sure I like the idea myself. The general point is that there is a risk we diverge in complete bike-shedding land rather soon, and ways to make incremental progress before we like all the names could be helpful.)

[shindere]

Gabriel Scherer (2020/06/12 02:19 -0700):

Note: with a list of names for each warning, we have the option to *not* offer names for some warnings if we don't agree on a good name. So we could keep @nojb's implementation work, but start with only a handful of names that people like, and keep numbers-only for some warnings. (I'm not sure I like the idea myself. The general point is that there is a risk we diverge in complete bike-shedding land rather soon, and ways to make incremental progress *before* we like all the names could be helpful.)

Well it feels odd to me too in terms of user experience to provide names for some warnings and not for otehrs, although I also understand very well the risk there is in waiting that we never agree.

…

[Octachron]

Personnally, I think that this an excellent idea. I will try to make few name suggestion later today.

[bschommer]

One thing one might consider when adding names to warnings is to add a logic similar to the one of gcc concerning unknown warnings:

When an unrecognized warning option is requested (e.g., -Wunknown-warning), GCC emits a diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other diagnostics are being produced. This allows the use of new -Wno- options with old compilers, but if something goes wrong, the compiler warns that an unrecognized option is present.

[Octachron]

Where are we for this PR? The discussion seems to have died without falling in endless bikeshedding. Does this mean that all names are consensual enough, even No_unsafe_array_syntax_without_parsing ?

[nojb]

Where are we for this PR? The discussion seems to have died without falling in endless bikeshedding. Does this mean that all names are consensual enough, even No_unsafe_array_syntax_without_parsing ?

Good question. My feeling is that the systematic assessment that @lpw25 suggested (and @fpottier seconded) has not taken place yet. @gasche had suggested starting with only naming a subset of warnings, but I don't think that is a good idea: users will then resist using names for warnings that were not in the initial set in order to be as compatible as possible, which will defeat the purpose of the patch.

I am not really sure how to proceed here...

[Octachron]

My systematic assessment is that the proposed names are serviceable. I can write down my evaluation if it would help.
But if we want to go further, I think that one good option might be to hold a meeting where all attendees go over the whole list of warnings.

[gasche]

Meetings? Yuck. I propose to go ahead based on the systematic review of @Octachron. If @nojb is willing to update the warning constructor names to follow the current proposal, that part of the PR is good to go. We pinged all maintainers already, and interested people had more than enough time to voice concerns if they have some.

The part that needs review is the overall implementation (I remember looking at it and being fine with it, but I don't remember the details), and especially ensuring that we have enough mechanisms to evolve gracefully when we (1) add new warnings (maybe we should implement @bschommer's suggestion for example) and (2) rename an existing warning (the old name should still be recognized for compatibility reasons).

[nojb]

Meetings? Yuck. I propose to go ahead based on the systematic review of @Octachron. If @nojb is willing to update the warning constructor names to follow the current proposal, that part of the PR is good to go. We pinged all maintainers already, and interested people had more than enough time to voice concerns if they have some.

The part that needs review is the overall implementation (I remember looking at it and being fine with it, but I don't remember the details), and especially ensuring that we have enough mechanisms to evolve gracefully when we (1) add new warnings (maybe we should implement @bschommer's suggestion for example) and (2) rename an existing warning (the old name should still be recognized for compatibility reasons).

I rebased the PR on trunk and did the renamings. Regarding @bschommer's suggestion, I think it is a good one but it is orthogonal to the present PR (the same logic can be applied to the numeric warnings). So I would prefer to leave it for a follow-up PR.

[gasche]

I believe that the two proposals are mostly identical, except that (1) alert payload don't currently allow spaces, they probably should for readability for the same reasons and (2) you propose to forbid - in warning names and use _ instead. In particular, if we don't deprecate or disable the just-a-letter syntax, with your proposal we would still have the problem of an ambiguity between parsing -foo as disabling the alert/warning foo and as -f o o for example.

Extra remarks:

• In principle I agree that using the same syntax for both features would be nice.
• There are two reasons to avoid dashes (-) in warning/alert names. The first is to reduce ambiguities in the syntax, the second is the form [@@alert foo "Reason why we alert here"] which requires a valid OCaml identifier for foo, so no dashes here.
• One fairly strong reason to use dashes in warning names is that this is what the vast majority of languages use (in particular: C, Scala, Haskell). On the other hand, it appears that Rust uses underscores as you suggest, making it a Cool Choice.
• Now that I read the documentation, I don't like the way the alert attribute is overloaded for two different things, with [@@alert foo "bar"] and [@@alert "bar"] being interpreted in very different ways. I think the former syntax should use have used an @@alert_on_use attribute instead. This is mostly orthogonal to the discussion, but there is also interest in having a @@warning_on_use construction for warnings (warnings affecting the use-site of items rather than their declaration-site).
• (unrelated) I found out that the just-a-letter syntax for warning specifiers is not documented in ocamlc -help, only in the manpage and the manual. We should not hesitate to deprecate it: it is likely that few users know about it.
• I still believe that the documentation of alerts is unclear. It should do as for warnings, explain +<foo>, -<foo> and @<foo>, and possibly later define ++<foo> and --<foo> as special options for odd cases. Even if we mirror the alert behavior, we should not reuse the confusing description.

[xavierleroy]

Do I understand correctly that this is another new feature in 4.12 that was merged 6 months ago and is being redesigned at the last minute and will therefore have to be pulled out the 4.12 beta urgently? What does this says about our design process?

[gasche]

To clarify: most of the feature is fine, the problem is with the current support in @@warning attributes. I do believe that this parts requires a redesign, after using it once.

[Octachron]

We have an user interface that is lacking a natural extension due to backward compatibility constraints. It is far too late to add any extension to the user interface at this point of the release cycle, without even mentioning breaking backward compatibility.

If the attribute warning cannot support list of named warnings, would it not be simpler to add a @@@warnings version with a clean interface?

[gasche]

For me the problem is not so much that passing several warnings is not supported, it is that @@@warning "-labels-ommitted" appears to work and is doing something completely different and subtly wrong. This was already the case before @nojb implemented this feature -- this attribute disables all warnings including on older OCaml versions -- but the problem is that now users are infinitely more likely to write this confusing input.

Also, I am not convinced by the "this is too late in the release cycle" argument. If we don't fix the issue now, users are going to get confused by it down the line, and they will open issues and ask us to fix it. "After the release" is actually later than today in the release cycle, and it will be orders-of-magnitude more costly in terms of total work. (I don't want to be sensationalist, this is an issue about warning attributes; but note that giving an easy way to mistakenly disable all warnings is a believable bug-inclusion vector). We should fix it now. As to "why wasn't it caught before?", I suppose that it is the kind of issues that is hard to catch without dogfooding.

[gasche]

Another way to say things is: I am a user of 4.12~alpha and I am reporting a problematic usability bug. I don't believe that "it is too late in the release cycle to fix this issue" is an appropriate response.

[Octachron]

It does not make sense to not open an issue indeed. And if we can find a fix that alleviates the issue, does not break backward compatibility, and is consensual; then it makes sense to integrate it.

The issue merely does not seem acute enough to break backward compatibility in a hurry or to block the release if a satisfying design seems out of reach.

[alainfrisch]

In principle I agree that using the same syntax for both features would be nice.

I was not only suggesting to use the same syntax, but even the same command-line options and attributes (only for warning mnemonics). The -w/-warn-error options and corresponding attributes would be for (legacy) numeric/single-letter; and the alert-related options/attributes would be for named warnings.

[gasche]

The core of my proposal is to deprecate just-a-letter warning specifiers. This does not break backward-compatibility. If you write [@@warning "-foo"], everything works as before, but you get a deprecation warning that explains that just-a-letter-specifier like o are error-prone (they can be confused with full warning names) and that you should write [@@warning "-f-o-o"] instead if this is what you mean. The suggested form is also backward-compatible).

[gasche]

@alainfrisch so you propose to un-support -w -partial-match, and instead support -alert -partial_match. I find this less discoverable, but maybe other people like the idea? That means using the same namespace (so warning names and alert names may conflict with each other), which may be appropriate to avoid misunderstandings but also means that adding new compiler warning names may suddenly shadow user-chosen alert names, which sounds annoying. (We have some poor namespace mechanism for attribute names, but not for alert names, if I understand correctly.)

[Octachron]

Alerts and warnings have a quite different semantic nowadays. Mixing the two seems confusing since the reader needs to guess which alerts in -alert are in a fact warnings to understand their potential effects.

Deprecation is very much a backward compatibility issue right now with dune default settings. And the issue might be even more problematic if we mix meta-level deprecation alerts with the usual deprecation alerts. For instance, how do I disable globally on a project these warning deprecation alerts?

[gasche]

We could get a sense of whether anyone is actually affected (uses just-a-letter warning specifiers) by implementing the warning and checking the opam-repository. Should we?

For instance, how do I disable globally on a project these warning deprecation alerts?

In an imaginary world where this feature is implemented, you would need to disable warning 3, so either you change your build system to pass -w -3 before other warning-related options, or use the corresponding OCAMLPARAM setting (the "before parsing command-line arguments" part).

[Octachron]

But then I am disabling all the useful deprecation warnings when I only wanted to remove the warning for

[@@@warning "@imported-from-the-future"]

[gasche]

Yes, but you only need to do this as a stopgap measure until you fix your just-a-letter warning specifiers to use a + or -. Are you suggesting that we should add a new warning instead of the deprecation warning? I'm not convinced -- it adds more complexity and I don't see clear benefits -- but I could implement this if you insist.

[alainfrisch]

Alerts and warnings have a quite different semantic nowadays. Mixing the two seems confusing since the reader needs to guess which alerts in -alert are in a fact warnings to understand their potential effects.

Which semantics difference do you see between alerts and warnings? It's already the case that warning 3 is actually mapped to an alert.

That means using the same namespace (so warning names and alert names may conflict with each other), which may be appropriate to avoid misunderstandings but also means that adding new compiler warning names may suddenly shadow user-chosen alert names, which sounds annoying. (We have some poor namespace mechanism for attribute names, but not for alert names, if I understand correctly.)

That's right, but this also applies to alerts that could be added to the stdlib (or to specific language features).

[Octachron]

Since alerts are currently a quite specific kind of warnings (compiler messages?), after seeing -alert @some_tag, one can know that some specific functions and modules should be avoided, whereas a warning might have a broader scope.

[damiendoligez]

I can't belive this conversation died without reaching a conclusion.

I agree with @gasche that the long-term solution is to deprecate the single-letter syntax, but we can't rush such a change and repeat the Pervasives fiasco.

As a stop-gap measure, I propose that we warn when the single-letter syntax seems to be used, but the set of letters contains either a repeated letter or 'a' along with some other letter(s). Since these combinations are redundant, they are unlikely to be used in the wild. On the other hand, all the new warning names (and probably most of their misspellings) are caught by it.

[Octachron]

Alerting on redundant warning letters does seem like a good starting point since it matches the current interpretation.

[gasche]

I can't believe this conversation died without reaching a conclusion.

Apologies (I'm sure you didn't mean it as criticism but I wondered about how this happened as well); I think I grow a bit discouraged by the impression of being alone in caring about the issue.

I agree with your proposal to warn on redundant warnings as a first step; but I think that our user-interface will not be pleasant until we decide to stop considering only-a-letter specifications as things user should write.

But: I think that deprecating only-a-letter specifiers is not a Pervasives-deprecation-like situation. The big difference is that there is a backward-compatible fix to silence the warning, which is to use [+-]<letter>, which is supported already today. This means that people can upgrade their build systems to fix the issue without any downside. This was not the case with the Pervasives->Stdlib transition (adding a dependency on a compatibility shim was considered problematic by many users).

[damiendoligez]

I think that deprecating only-a-letter specifiers is not a Pervasives-deprecation-like situation.

You're right. But I'm still worried about annoying every dune user out there.

[gasche]

Does dune use just-a-letter specifications in its default warnings?

[dra27]

@gasche - no, Dune doesn't. Dune's default flags are defined here; there are three warnings sets:

• Vendored mode: -w -a
• Release mode: -w -40
• Dev mode: -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40

so the only mnemonic is a and it's used "correctly".

[dra27]

I think therefore Dune users would be affected the same as any other - manually-specified warnings flags would be caught by any deprecation.

[Octachron]

Rereading the issue, I am perfectly fine with deprecating single-letter with a specific alert (ocaml.deprecated-cli?) in 4.13. Or if we need a partial fix in 4.12.0, alerting on redundant letter-specifications.

On the issue of having multiple named warning by arguments, I think it would clearer to have a distinct separator (space or comma?) rather than a longest-match specification with the aim to make parsing independent from the list of known warnings.

[gasche]

Let's start by implementing the deprecated-cli alert you suggest in trunk/4.13. Most of the code will be needed anyway if we decided to go for a more-restricted 4.12 palliative.

[Octachron]

Ok, let me do that.

[damiendoligez]

continued in #10207