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

Suggestion: mnemonic aliases for warning codes #7823

Closed
vicuna opened this issue Jul 16, 2018 · 5 comments · Fixed by #9657
Closed

Suggestion: mnemonic aliases for warning codes #7823

vicuna opened this issue Jul 16, 2018 · 5 comments · Fixed by #9657

Comments

@vicuna
Copy link

vicuna commented Jul 16, 2018

Original bug ID: 7823
Reporter: @mjambon
Status: new
Resolution: open
Priority: normal
Severity: feature
Category: misc
Monitored by: @nojb @gasche @alainfrisch

Bug description

The identifiers of warnings emitted by the OCaml compilers are currently numeric and require looking them up whenever they're encountered in code. Having numeric IDs is fine when a warning takes place and it's reported by the compiler with a description of the problem. But in other cases, having only a numeric code is unhelpful and requires the reader to look it up. These situations include:

  • Instructions to the build system which override the default set of warnings or errors e.g. FLAGS = -w +42
  • Inline annotations to locally disable a warning e.g. [@@@warning "-56"]

One solution is to have the programmer add a comment reminding the reader what the warning is about, but it's a bit of work for the programmer to copy-paste the description of the warning or make up their own. Instead, I'm suggesting that English-based aliases be provided as an alternative to numeric warning codes.

I understand there's a whole art involved in creating aliases that are not too long and yet sufficiently helpful in remembering what they mean. For instance, perhaps warning 42 "Disambiguated constructor or label name." could be aliased to --disamb-cons.

In terms of syntax, we could mix numeric and non-numeric codes as in -w +41+-disamb-cons--something-else. Or maybe -w +41+:disamb-cons:+:something-else:. Visually, I find -w +41+{disamb-cons,something-else} more satisfying but it doesn't play well with unix shells. Yet another option, since there's not much value in supporting a notion of range with non-numeric identifiers, is to have one command line option for each warning that we want to add or remove: -w +disamb-cons -w -something-else would be used to turn on "disamb-cons" and turn off "something-else". In this case, a lowercase letter following a plus or minus is what would indicate the beginning of a multi-character warning identifier. For inline overrides, perhaps then we could have [@@@warning "+disamb-cons -something-else +15..20-18"].

@vicuna
Copy link
Author

vicuna commented Jul 16, 2018

Comment author: @gasche

I've been thinking about implementing names for warning on-and-off for the reasons you give, and support the idea.

While their use in attributes shouldn't make the code look difform, I wouldn't give too much weight to concision either in these warning naming choices (I guess I'm just not a concision person in general). I think they are more important for people reading the code than for the people writing it.

@vicuna
Copy link
Author

vicuna commented Jul 17, 2018

Comment author: @alainfrisch

I also support the idea. Warning messages should display the symbolic names to make them more discoverable.

Also, see #1804 . "alerts" will be identified by name, and it is planned that some language features could "raise" alert themselves. The real remaining difference with (named) warnings is that alert names are not predefined, which prevents catching typos.

@github-actions
Copy link

github-actions bot commented May 7, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 7, 2020
@github-actions github-actions bot closed this as completed Jun 8, 2020
@mjambon
Copy link
Contributor

mjambon commented Jun 8, 2020

I think this issue is still valid and shouldn't be closed.

@nojb nojb reopened this Jun 8, 2020
@nojb nojb removed the Stale label Jun 8, 2020
@nojb
Copy link
Contributor

nojb commented Jun 8, 2020

For the CLI interface we could go the way of the C compilers/clang, and use specific flags -Wfoo, -Wno-foo for each warning, see https://clang.llvm.org/docs/DiagnosticsReference.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants