Add warning categories

[JohanEngelen]

This PR splits warnings into categories, which can then be disabled/enabled to suit the user. For example the "statement not reachable" warning is very hard/ugly to work around in generic code, which means the user cannot compile with warnings enabled (warning spam), nor with warnings=errors. With this PR, the user could do dmd -w -Wno-not-reachable, enabling all warnings except the not-reachable warning category.
The dlang.org Warning page says:
"Most have generated some spirited debate as to whether it should be a warning, an error, and what the correct way to write D code in these situations should be. Since no consensus emerged, they appear as optional warnings, with alternatives on how to write correct code."
This PR makes warnings individually optional.

New commandline options are added, all starting with capital W: -W...
The behavior of -w and -wi is unchanged.
-Werror (warnings=errors, without enabling all warning categories)
-W... and Wno-... where a category name must be inserted on the dots.
The category name is output at the end of the warning text (like clang does).
A warning can be put into more than one category, although at the moment all warnings belong to a single category.
Later options overwrite earlier options.
Examples:
-w, as before.
-wi -Werror == -w
-wi -Wno-conversion, will not warn on int += float
-Wconversion, will only warn on int += float
-Wno-conversion -Wconversion, will only warn on int += float
-Wconversion -Wno-conversion, will not warn at all
-Werror -Wconversion, will only warn on int += float and will error on that warning

Some PR details:

1. move warning-related code from errors.d to the new file warnings.d
2. define a number of categories and assign each of the current warnings to a category (most are in their own category), implement category commandline arguments + filtering of the warnings, add category name (colored) to the warning output
3. add extra tests for warnings. In the process of adding warnings, I fixed a bug where post-semantic warnings did not result in an error with -w (notably, the previously untested toHash() warning).

[JohanEngelen]

Note that the CyberShadow/DAutoTest fails because of compilation with -w used to pass without fail upon Ddoc warnings...

[JackStouffer]

I am in favor of the changes in this PR, I can't comment on the implementation details however. Because this is such a large change, I would suggest posting something about this on the news group to solicit more opinions.

[WalterBright]

I'm afraid what this does is balkanize D into multiple languages. It complicates the compiler, the spec, tutorials, and is something we'll have to support for the foreseeable future. It is not worth it.

I know you've put a lot of time into this, and I'm sorry about that. Perhaps in the future a DIP would be worthwhile first to see if the idea has fertile ground.

[JackStouffer]

People almost never reply to DIPs seriously though. This at least elicited a definitive response from you :)

[JohanEngelen]

@WalterBright (obviously a very disappointing response) I don't see how this complicates the compiler so much.
The spec explicitly says: "Warnings are not a defined part of the D Programming Language. They exist at the discretion of the compiler vendor, and will most likely vary from vendor to vendor." So I don't see how this would complicate the spec or tutorials.

[CyberShadow]

I think the issue here is that such a switch affects not only the code it is meant to affect, but also all included modules from all included libraries. Some libraries may begin requiring that you turn some warnings off, thus complicating the build process for users of that library.

Perhaps a more reasonable approach would be pragmas which disable some warnings locally, and have the scope of at most one module.

[WalterBright]

obviously a very disappointing response

I understand, I don't like seeing work go to waste. That's why I suggested in the future finding support for an idea beforehand.

I don't see how this complicates the compiler so much.

To start with, 400 lines of changes. And it opens the door to this style, so I'd expect a bunch more coming.

I don't see how this would complicate the spec or tutorials.

Somebody has to document this stuff, explain it, determine best practice, etc.

On a more global level, warnings are a failure of language design. Switch selectable language behavior is just a bad idea. I know that we have some already, but every one of them is symptomatic of failure.

[WalterBright]

Perhaps a more reasonable approach would be pragmas which disable some warnings locally, and have the scope of at most one module.

Instead of pragmas, I'd prefer there be ways of writing the code to avoid the warnings.

[CyberShadow]

Instead of pragmas, I'd prefer there be ways of writing the code to avoid the warnings.

Any ideas about the unreachable code warning in templated code?

There is currently a proposal to remove that warning entirely (to which I'm opposed): #5229

[JohanEngelen]

Somebody has to document this stuff, explain it, determine best practice, etc.

Pretty much all warnings are not documented at the moment (yet they were added, and a few are not tested either).

[JohanEngelen]

On a more global level, warnings are a failure of language design. Switch selectable language behavior is just a bad idea. I know that we have some already, but every one of them is symptomatic of failure.

Perhaps it indicates a failure of language design, but that's what D is then and users have to deal with it in some way. Clang has some very nice warnings that involve coding style, that apply to D aswell. For example,

• a warning on code that relies on operator precedence, but is easily interpreted wrong by humans: if (A && B || C && D) Such warning is fixed by adding parenthesis.

- warning on if(a = someThing()), fixed with adding extra parens.

I am pretty certain that these two examples are not going to be resolved by changing the D language. In such cases, warnings help early prevention of bugs (imho). Of course this is debatable, therefore they are made optional.
I wouldn't call this "switch selectable language behavior", but rather "switch selectable compiler diagnostics" which is pretty neat I think.

Edit: Thanks @CyberShadow for the correction about if (a = ...).

[CyberShadow]

Clang has some very nice warnings that involve coding style, that apply to D aswell. For example,

It's not clear from your message, but in case you're not aware, D has the same warnings, which are - as you said - resolved by changing the code a bit (adding parentheses, or adding an explicit "== true"). This is what @WalterBright meant by "writing the code to avoid the warnings".

I wouldn't call this "switch selectable language behavior", but rather "switch selectable compiler diagnostics" which is pretty neat I think.

With warnings-as-errors, it is no longer a diagnostic, but a rule of what is allowed and what isn't - hence, a "different language".

[JohanEngelen]

D has the same warnings,

Is not exactly true: the first example is not a warning in DMD, and the second example is an error in DMD.
I know what Walter means by "writing the code to avoid the warnings", but like dlang.org says: people debate about certain things.
If not warning about it, the plan is to change the D spec to disallow if (a && b || c && d) ?

[CyberShadow]

the first example is not a warning in DMD

That's surprising, it probably should be.

If not warning about it, the plan is to change the D spec to disallow if (a && b || c && d) ?

We can't introduce that as an error right away, as that would break code. It could be introduced via a deprecation cycle.

people debate about certain things

That's a poor excuse for the problems that fragmentation brings with it. I agree with @WalterBright here.

[tsbockman]

Cybershadow has it right:

I think the issue here is that such a switch affects not only the code it is meant to affect, but also all included modules from all included libraries. Some libraries may begin requiring that you turn some warnings off, thus complicating the build process for users of that library.

For the "statement not reachable" warning (DMD 14835), Phobos itself would eventually require disabling the warning. I don't think that's a road any of us really wants to go down; there has to be a better solution.

[JohanEngelen]

One could treat code in system headers different from user code (like clang does).

[CyberShadow]

One could treat code in system headers different from user code (like clang does).

How is that implemented? Not by locally disabling certain warnings using pragmas?

[JohanEngelen]

http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers

I will no longer maintain this PR. The PR is a solution to a current problem, with no other current solution, but to me it is clear you guys don't want this. Perhaps you can use parts of it. At least you may want to apply the bug fix.

[CyberShadow]

http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers

Oh, I misunderstood what you meant by system headers (I thought you meant the headers supplied with the compiler, not the system). That is not a problem that D has, since DMD does not have a necessity to be compatible with some pre-existing system headers that were written before it existed.

This is a poor parallel to Phobos, of course, since as we are in control of Phobos as much as we are in control of the compiler, we would just change Phobos instead of introducing compiler switches.

I will no longer maintain this PR. The PR is a solution to a current problem, with no other current solution, but to me it is clear you guys don't want this. Perhaps you can use parts of it. At least you may want to apply the bug fix.

Do pragmas to disable the warnings locally not solve the problem you have?

[JohanEngelen]

(I thought you meant the headers supplied with the compiler, not the system)

I did mean that :) Afaik, "system headers" includes the C++stdlib headers.

This is a poor parallel to Phobos, of course, since as we are in control of Phobos as much as we are in control of the compiler, we would just change Phobos instead of introducing compiler switches.

Yep, I thought so too.

Do pragmas to disable the warnings locally not solve the problem you have?

Yes pragmas would work. But I am discouraged from implementing it. This PR could be a first step if someone wants to take it up.
For what I am working on, a fork of LDC is used. So I can just remove the particular offending warning. I assumed more people would have similar problems, but would not have the luxury of a forked compiler and would benefit from an improved warning system.

[WalterBright]

For what I am working on, a fork of LDC is used. So I can just remove the particular offending warning.

I'm curious which warning is causing you so much grief.

[dnadlinger]

@WalterBright: If I'm not mistaken, this is the forum discussion, which links to this original thread. The issue is with "statement not reachable" in generic code, which, as @tsbockman points out, will get worse if we improve VRP.

[WalterBright]

Thank you. I replied in the original thread.