Hacker News new | past | comments | ask | show | jobs | submit login
Useful GCC warning options not enabled by -Wall -Wextra (kristerw.blogspot.com)
288 points by rbanffy on Oct 4, 2017 | hide | past | web | favorite | 166 comments



Nice summary. Out of curiosity:

Why are those warnings not automatically included in "-Wall -Wextra"? Is there any issue with enabling them? Are these somewhat experimental?

How do these GCC warning compare to Clang warnings? Do these warnings exist there, too?


> Why are those warnings not automatically included in "-Wall -Wextra"? Is there any issue with enabling them? Are these somewhat experimental?

Because tons of people keep complaining "muh my build breaks if they enable new warnings". Dammit, idiots, if your build break there's a good chance your software is already broken too; that's the point of breaking the build: making bugs visible so that you have to fix them.


That's why you don't enable -Werror in release builds, contrary to received wisdom on the subject.

When you compile with -Werror, you surrender control over your build's success to the whims of the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior. The personal opinions of future compiler authors shouldn't affect whether your program builds successfully. Sometimes warnings are bogus.

Sure, use -Werror in development. But don't make it the default.


> The personal opinions of future compiler authors shouldn't affect whether your program builds successfully.

So, let's say that you find some 2003 software on some deep corner of SourceForge that you want to use.

Would you rather :

* Have the build fail because new warnings were introduced ? * Have the build succeed and then get crashes at runtime because the behaviour of the compiler changed anyways and the program was rendered invalid ?

I'll personally always want the first one.


There's a name for a kind of change that makes a previously correct program incorrect: "compiler bug".


> There's a name for a kind of change that makes a previously correct program incorrect: "compiler bug".

No. The only thing that matters is that the language rules are respected. Or maybe you'd want programs that were correct with the first C compilers to still work today ? Because for instance they didn't do type checks at all. `void f(float, float); int x = f;` you say ? no problem! `char c; c[-15];` ? no problem! Everything did compile, sure. Is it the world we want ? Most certainly not.


Trigraphs used to be part of the standard and are now removed. So really old programs that used them now would fail to compile. Granted, that is a bit of a low hanging fruit.


Also "horribly written code."

I've been playing around with some code from 1993---the Viola web browser (one of the first graphical web browsers). Ooh boy was it fun when I ran it with "-pedantic -Wall -Wextra". Hundreds, if not thousands of warnings. A ton of unused argumetns, implicit declarations (no prototypes), unused variables, return type not specified, a few "control reaches end of non-void function," some format string mismatches. Pretty much what you would expect from K&R C (the code paid some lip service to ANSI C, but not a whole lot).

Also, the code runs (not well, but it does) on 32-bit systems, but immediately crashes on a 64-bit system. That's because of the whole "int==long==ptr==32 bits" that permeates the code.


Who says the program was previously correct? Just because it compiled and didn't blow up so far, doesn't mean it's correct. It might or might not have been correct..


the program wasn't "previously correct", else a new, but still language compliant, compiler would build it just fine


Worth noting that some warning flags (at least when combined with -Werror* ) make the compiler not standard compliant.

* It's been some time since I last partook in language lawyering, and can't remember if standard compliant C compilers are allowed to produce diagnostics, the term used, for well-defined code as long as they also succeed.


Perhaps what we really want is a flag to cause the compiler to error for code that would produce undefined behavior and give warnings for the rest? Turning all warnings into errors is a great "worse is better" technique but it seems a bit too coarse-grained to me.


Alas, undefined behaviour in C++ and C is in general not that easily detected..


Oh, to be sure! However, my claim is that if compiler authors are able to add more no-false-positives undefined behavior warnings over time, I kinda want those to "break" the build for my existing software... But if something is just a style check or creates false results, I'd rather the build be allowed to happen.


It's true, but there's a few tools out there to help now, such as tis-interpreter (https://github.com/TrustInSoft/tis-interpreter)


Yes, an interpreter has a much better shot at detecting runtime undefined behaviour than a compiler.

The whole point of undefined behaviour in C and C++ is to let the compiler cheat: ie a Java or Haskell compiler would have to take into account that (i < i + 1) can sometimes be wrong for native ints, and would have to prove that overflow can't happen in order to optimize this comparison away to True. Undefined behaviour in the standard frees C and C++ compilers from these obligations, and they can just assume overflow for signed ints won't happen.

These shortcuts (plus a lot of smarts) make it feasible to write a fast optimizing compiler with the 1970s state of the art in static analysis.


Agreed. If the compiler wants to optimize something because of Undefined Behavior, that's something that -Wall should break on.

'Warning, you have undefined behavior', let the programmer decide what the intent of the section is and fix it.


Just FYI, not all undefined behaviour can be detected at compile time using current C semantics. I'm very very in favour of cracking down on undefined behaviour and changing things from undefined to implementation-defined and stuff like that, but it's not as easy as just flicking a switch and making the compiler warn whenever it assumes undefined behaviour won't happen.


Do you have an example I could take a look at? I'm actually not well versed in the C world


A simple case is the warning clang will generate for:

  int err = 0;
  if (err = f()) {
  }
This is indeed a useful warning, as it's common to mistype == as = in this situation, but it's also absolutely correct code, and the idiom that silences the warning (surrounding with extra parentheses) does not change anything about the correctness of the code.


A hint could be that you're initializing to zero a local just to overwrite it. Not an error or a warning but it could hint that you did not mean to, isn't it ?


The initialization is just an example to show that err is a previously declared variable. The actual code could easily be different, but the warning is emitted on seeing the assignment in the conditional.


Unintended code (like having both arms of an if-else-statement execute the same action, or a duplicate condition in a logical expression) is sometimes perfectly valid as far as the standard goes. A double condition for example may simply be a result of poor refactoring, and thus the program even works as intended – however, the compiler cannot know that. That's why the helpful warnings flag those statements – -Werror then makes the compiler abort the compilation altogether on any warnings which is clearly not standard-compliant in cases like the one I outlined.

The majority of the warning options showed in the article are intended to find unintended code (which can however perfectly defined if not intended behaviour as opposed to invalid, undefined behaviour). The very basic examples showed for those options also themselves do not contain anything that by itself would make the code not standard-compliant.


Well, the standard does not say that if you ask the compiler to flag potentially suspect code that it should compile successfully. That doesn’t make it non standard if you decide you do want your compiler to stop compiling when it encounters potentially suspect code and enable that mode wth a conpiler flag.


The opinions of (future) compiler authors decides already wether your programm will work in all cases. With those warnings they indicate some of their decisions.


Future compiler authors are at least in theory beholden to current language specifications. If a program does something legal that a compiler author feels is a bad idea, the compiler can whine all it wants, but it still has to compile the program. Compilers drop support for older specifications far less often than they add new warnings.


This is especially true for warnings about && and || precedence. When you use those two operators in one expression without parentheses, IIRC both gcc and clang complain, but I once wanted to compile some software that thought it was a good idea to -Werror all the code and don't use parens around &&. Fun times fixing all the zillion makefiles that mentioned Werror. (Yay autoconf?)


That's the one warning that I like to disable. I hate having to write superfluous parentheses around conjunctions. I mean, come on. It's just as lame as requiring them around multiplications would be.


Even if /you/ know the order of operations, making them explicit with the parentheses provides clarity and allows future maintainers to quickly (and correctly) deduce the structure of the logical construct.

I generally only leave them out in short simple to follow 'normal math' order of operations.


Sure. The rule you suggest might be good coding practice --- but people can disagree, and their omitting the extra braces doesn't make the code wrong. An existing correct program shouldn't one day stop building because someone added a warning that enforces your preferred operating precedence marking style.


That is because the compiler authors conflated warnings with a linter. Warnings should be used to indicate basically just two things.

1. Dead/Duplicated Code.

2. Probably Broken Code.

Anything else is stylistic and the realm of opinion. Even if it's informed opinion enforcing it belongs in a linter.


To be fair, a compiler contains all the infrastructure you'd want in a linter. I don't mind compilers having lint-like warnings: I just want them to very clear about the differences between these warnings and warnings that reflect functional problems with high probability.


What I’d like is for compiler authors to just admit this and include a lint mode.

There’s some practical and political difficulty with adding new warnings to a widely used compiler, partly because you don’t want to generate too many new warnings for existing code, partly because people compile with -Werror and upgrading the compiler will break (well, sprain) their build. Those difficulties would be easy to skirt by just tossing new warnings into lint mode and promoting them to compiler warnings if they turn out to be very useful in practice.


I agree. A -Wlint would be a great way to keep the distinction but reuse the compilers infrastructure.


The C standard is dropping support for things like gets, a compliant modern compiler should fail to build any code that uses that. This is very controversial with compilers - and gets is among the most horrid thing that ever made it into a compiler.


PulseEvent and IsBadReadPtr are worse. Windows has some of the very best and the very worst in API design.


In general yes, but the posts above were talking about a warning about undefined behavior, so the standard says that anything could happen in that case.


> you surrender control over your build's success to the whims of the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior.

You mean the people who write the specs of the language? Well you surrender control to these folks when you decided to pick this language!

Otherwise the problem about memcpy of zero-size and nullptr is unfortunate from compiler engineers (at least clang engineers) point of view. The story is that the compiler is optimizing based on programmer annotations: after all if a programmer takes the effort to annotate their APIs, let's benefit from this. However the GNU libc annotated `memcpy` (and others) with this non_null attribute, causing this fragility on invalid (according to the spec) but common code.

Opinion of clang authors is somehow "Deleting a null pointer check on p after a memcpy(p, q, 0) call seems extremely user-hostile, and very unlikely to result in a valuable improvement to the program".

Clang dev thread: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066....

Note in this thread that clang devs are trying to get the language changed to remove this UB.


I disagree. Rather write nasty rants in comments that explain compiler-specific workarounds. It should not be too many anyway, if one adheres to KISS.

Otherwise you surrender control over the correctness of your program "to the whims of the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior".

More debatable decisions will be made in compiler implementations and I'd rather force the user who compiles the program to understand the implications of warnings by turning them into errors than letting them ignore them (because "it compiled, after all!").

If I was to make the call, I'd make "-Werror" default in all compilers, remove the option and allow to disable it by "--I-really-want-to-ignore-warnings-for-fscks-sake" which when used would sleep(30) after an all-caps nag screen that tells people "don't ever use that option."


I have one project where I run with "-pedantic -std=c99 -Wall -Wextra". The one error that pops up is:

     warning: ISO C forbids assignment between function pointer and ‘void *’ [-pedantic]
But POSIX requires that behavior (the actual target of the build). So for now, it's benign, but if the platform changes (to a non-POSIX system) then yes, it will have to be investigated.


This affects many codebases. For example, looking up functions with `dlsym` triggers this warning which is probably one of the main reasons for the POSIX requirement. It's unfortunate that GCC doesn't allow to disable only this particular warning. When writing cross-platform code, `-pedantic` is too useful to disable completely.


Try using intptr_t


When I download some random unmaintained tarball from years back, I don't want to have to fight with the compiler. Just build dammit.


"When I buy some random unmaintained car from years back, I don't want to have to fight with the transmission fault mode. Just drive dammit."


The better analogy is: "When I pick up some blueprint for the building from 80s I don't want to have to fight with changed sensibilities of the foreman. Just build the thing dammit."


Software doesn't mechanically wear over time, this is an awful analogy.


Yeah, I admit. A better analogy on my part would have been that you buy a car and it's not street legal anymore, or one that does not work in today's environment anymore. "I don't want to add seat belts, just drive" or "I don't want it to run on unleaded fuel". (Note that I have no idea if there actually were cars that required leaded fuel.)

Anyway, I think even more likely than running into new compiler warnings/errors is running into external dependency issues, which require you to touch the software anyway, and, I'd argue, are often much harder to fix than fixing code that triggers warnings.


If you build old software that relied on a specific compiler treating undefined behavior in a certain way, then it could.


yes, people actively rewrite it in rust instead

More seriously, software certainly wears, not relative to itself but relative to the environment it is run in. It wears in the sense that active maintenance operations are needed to have old software perform as it should: installing old libraries, running it in emulators, etc.


You'd be surprised…


What are the odds that a tar-ball suffering bitrot will decompress cleanly?


What I had in mind was UB changing over time more than tarballs getting corrupted (eg. strict aliasing-related optimizations).

There's practically no chance for a corrupted gzip-compressed tarball to decompress cleanly (even if by chance it inflates without error, there's a CRC32 of the uncompressed payload at the end of the gzip stream).


> What I had in mind was UB changing over time

I don't want to keep posting this[1] in every C discussion, but when it's relevant, it's relevant. TL;DR A particular version of a compiler interpreted undefined behavior in such a way as to elide code that specifically tested for an error case to exit early because the error implied undefined behavior, so the test could never be reached without undefined behavior. If that sounds like circular logic, that's because it is.

The error happened to exist in a fairly specific version of clang shipped by apple, but not in any of the major release versions:

I was not able to reproduce this bug on other platforms, perhaps because the compiler optimization is not applied by other compilers. On Linux, I tested GCC 4.9, 5.4, and 6.3, as well as Clang 3.6, 3.8, and 4.0. None were affected. Nevertheless, the compiler behavior is technically allowed, thus it should be assumed that it can happen on other platforms as well.

The specific compiler version which was observed to be affected is:

$ clang++ --version

Apple LLVM version 8.1.0 (clang-802.0.41)

Target: x86_64-apple-darwin16.5.0

Thread model: posix

InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

(Note: Despite being Clang-based, Apple's compiler version numbers have no apparent relationship to Clang version numbers.)

1: https://news.ycombinator.com/item?id=14163111


> the same people who think that it's okay that memcpy(0, 0, 0) is undefined behavior.

I am curious about this particular example. How does that ever make sense, where do you have that?

I'd accept that it would be nice to have memcpy(p1, p2, n) with valid pointers p1 and p2 to still be defined when n=0, because maybe n is variable and might work out to be 0 in some cases, which you then don't need to treat specially. I don't know whether that case is defined or not.

But in the case where either p1 or p2 are NULL, the memcpy will be illegal no matter what n is, so if p1 or p2 are variable you always have to check them anyway, and to run into the memcpy(0, 0, 0) case you would actually have to special case n=0 when p1 or p2 were NULL, which makes no sense.

Unless 0 is actually a valid address (kernel code with identity mapping at 0, simple platforms), but then NULL becomes a whole different beast.


Because memcpy(x, y, 0) for any x and y is a no-op. Why should we have a special undefined case for x==nullptr || x==nullptr? In general, we shouldn't introduce unnecessary boundary conditions to APIs.


Because not making that case undefined puts additional and completely unnecessary burden on any implementation of memcpy? Now you have defined that memcpy(_, _, 0) is a no-op, so now the implementation has to make sure that it remains a no-op, which, if for instance a particular memcpy needs to perform an operation before the actual copy, may well involve adding conditionals to your implementation. For a case that a program will never sensibly run into (see my previous comment for why).


It's perfectly sensible to run into the case: imagine we're copying from a buffer that's allocated on demand. In the case where we haven't allocated the buffer, we have buf==nullptr and buffsz=0. Why should it be wrong to memcpy(other_buffer, buf, bufsz) without first checking whether bufsz is zero (and thus buf is nullptr)?

I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?

I think that in some cases, the claim that undefined behavior allows for better optimization is bullshit.


> In the case where we haven't allocated the buffer, we have buf==nullptr and buffsz=0. Why should it be wrong to memcpy(other_buffer, buf, bufsz) without first checking whether bufsz is zero (and thus buf is nullptr)?

That's not enough. For the case that resolves to memcpy(0, 0, 0), other_buffer is also 0. That means you need to guard against memcpy (0, buf, buffsz) with buff/buffsz != 0 somewhere anyway. Unless you are saying that the standard says that the more general memcpy(p, 0, 0) is undefined, which is a slightly different discussion.

> I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?

In any world where you need to e.g. lock the destination memory before accessing it, or do any other preparation/teardown. Not all platforms out there have one linear address space. In small scale applications, as just one example, it is not uncommon to have different pointer types for different types of memory, e.g. internal vs. external memory, I/O address spaces (lots of side effects there), maybe even a non-volatile memory address space.

In my opinion, it is easy and sensible to declare that memcpy(0,0,0) shall be undefined, especially because, again, no sensible program should ever reach that case without error (which, again, is different from the memcpy(p1, p2, 0) with p1/p2 != 0 case which can be sensible reached).

In fact, I'd argue that your proposal is the one that introduces the special case that all implementations would now have to take care of, adding complexity at best and a performance impact at worst, for something that a program won't ever do.


Can you cite one example of a modern platform that would be meaningfully impacted by making memcpy(a,b,0) well-defined for all a, b? These hypothetical situations you're talking about aren't real. We don't have computers with 18-bit bytes or one's complement arithmetic either.

As for the distinction between a==0&&b==0 and a==0||b==0: first of all, the standard does make memcpy(a,b,0) undefined when a==0||b==0. Second, it's perfectly reasonable to run into a situation where both and b can be zero: consider the same demand-allocated buffer case. Why wouldn't you also want to demand-allocate the destination buffer?


In my opinion, it is easy and sensible to declare that memcpy(0,0,0) shall be undefined, especially because, again, no sensible program should ever reach that case without error (which, again, is different from the memcpy(p1, p2, 0) with p1/p2 != 0 case which can be sensible reached).

It's common to see valid but semantically-meaningless constructs like that in autogenerated C code. Same with quite a few other cases mentioned in the article.

It's just not a good idea for a compiler author to arbitrarily start throwing warnings for memcpy(a,b,0) at this late date, regardless of a and b. That ship has sailed, leaks and all.


You've phrased the result of the UB in a way that disguises the benefit: your complaint is adding a special case, and for an arbitrary memcpy(x, y, n) call, the compiler would have to provide it isn't in this special case (i.e. n != 0) or else it couldn't assume x != nullptr && y != nullptr. The codegen benefit isn't when n == 0, but when n is unknown.


Huh? What special case? When n is zero, we copy no bytes. That's the opposite of a special case: it's a general rule!


This is not how an assembly programmer sees this problem. (memcpy is often hand coded in assembly).

This means the code takes advantage of CPU pipelines and branch predictions, which means the source/destination pointer might be de-referenced and loaded into a register before the number of bytes is being inspected, or the assembly is executed with a 0 value, which still makes the CPU do a load or store on the src or dest pointer. This is being done to gain performance, and doing things in another order or explicitly validating the input decreases performance.

If then the src/dest pointer is NULL, the above scenario plays out badly, but in order to not sacrifice performance, the burden is placed on the caller to correctly use memcpy() instead of memcpy() detecting invalid use.


For memcpy(x,y,n), x and y can refer to the last byte on a page. What are you going to do, prefetch that one byte?


The rule is memcpy(x, y, n) is undefined if x or y are null. Making it defined if n is zero is a special case inside that rule.


Sure: except that rule isn't useful. You can't _rely_ on undefined behavior, so preserving the rule that "memcpy(x, y, n) is undefined if x or y are null" isn't helpful. OTOH, preserving the rule that memcpy(x,y,n) is defined for all x,y when n==0 really is useful.


It is useful. I'm focusing on the effect on code generation: the compiler relies on that rule to deduce that x and y are not null. The special case for n==0 makes it harder for the compiler to deduce the non-null-ness and thus harder for it to optimise based on that. Restricting when optimisations can be applied is the effect of your special case.

You might not agree with this as a design decision but I'm not arguing that this is the right trade-off, just answering:

> I don't think your optimization actually exists. In what possible world can we generate better code by assuming that for memcpy(x,y,n), when n==0, x!=nullptr||y!=nullptr?


But memcpy(0, x, y) is illegal for any x and y.

In mathematics, this is called a discontinuity. Google the value of 0^0 for comparable discussion (it ‘is’ 1 because that makes more formulas look nice)


The fundamental question can be summarised thus: "If you don't access an invalid address, is it still invalid?"

By my common sense, the answer is no.


I agree that using `-Werror` in release builds is a bad idea. But I don't think you need to "not make it the default" for this: just provide an easy way to disable it, and do it automatically for release builds. It should be on by default for any developer.


I used to have it on by default, but I found it getting in my way while developing. I'd do things like add some temporary debug code, or comment out part of an algorithm and then end up with a bunch of compile errors to fix. The compiler rightly points out that the code is of poor quality, but that's because I'm not done yet.

Warnings in that case are still useful. If the project is otherwise warning-free then it's easy enough to spot the new warnings and judge if they represent a real mistake.

My preference today is to set -Werror on CI builds. Any code that's final should be warning-free, but it's unnecessarily painful for work-in-progress.


In Java, the compiler complains about unreachable code and aborts. I think that's ridiculous. Sometimes it's useful to create unreachable code during development.

  int foo = doSomething();
  return 0;  // Debug XXX DONOTSUBMIT
  int bar = doSomethingElse();
  return foo + bar;
It's hilarious, though, that this workaround works:

  int foo = doSomething();
  if (1 > 0) return 0;  // Debug XXX DONOTSUBMIT
  int bar = doSomethingElse();
  return foo + bar;
Go's mandatory warnings are equally annoying. I'm sick and tired of little rarefied groups of language designers trying to impose their ideas about best practices on the rest of the world.


What did you expect from a language that dictates to where to put your opening brace.


What? That's a common style choice, not a language requirement.


You can't put the opening brace of a block on a separate line in Go, because the parser will insert a semicolon before it.


Sorry - I thought the conversation was about Java.


Why should it? I just don’t get the obsession. As long as no one commits any warnings (and it’s easy to have a no warning policy), then I don’t see why it should be enabled. Sometimes I just quickly wanna throw in an unsafe printf here and there, or not change a type in every place when iterating and trying changes. It’s just strange to disallow yourself to try quick changes that warns because they might be unwanted in certain situations.

And don’t worry, if the new idea I’m trying is worth it, I will fix it up so it doesn’t have any warnings. And I saved a lot of time by not fixing all these warnings on the previous 2 ideas I tried.


Because painful experience has shown us time and time again that if a developer doesn't have it on all the time they will not let issues creep in. In theory you can use a CI system which does have it on, but those are not universal.


Or... you can just fix the warnings before commiting. We have no tolerance for warnings on pushed code, and this has seriously never been a problem.

It’s just annoying to have to remove every unused variable/parameter when trying changes out.


Then use -Wno-* to disable specific warnings that are problematic, when they become problematic. That solves 90% of the issues with using -Werror.


What's warning on one platform is usually error on another.

People that decide what memcpy does, have vastly different problems in mind (and hardware realities) from people complaining about it when using it on <random platform>.


Using -Werror in development is annoying too. It works if everyone is running exactly the same OS, header files, and version of the compiler.

If not... different developers have different errors.

I try to build with all of the warnings turned on, multiple static analysis tools, and with multiple OS / compilers. Yet every time a compiler or OS is upgraded, there's a whole suite of new build warnings.


Have you tried this lately - in the last 10 years? things used to be bad, but clang and gcc at least have really cleaned up their act, most warning are now zero false positives and so you want to clean everything up. The only time I see the issue you described was when someone upgraded the compiler, and the new warnings were all real bugs.

Static analysis tools are a different problem: I find that most of them have so many false positives they are not worth looking at.


> Have you tried this lately - in the last 10 years?

I'm wondering what it is about HN that lets people be condescending know-it-alls.

If you look, there's only one of me online. I write between 10K and 20K lines of C a year.

So yes, I have tried this in the last 10 years.


Quite a few of the warnings not enabled in -Wall or -Wextra fail spuriously on good code. In combination with -Werror, new failures on perfectly fine code is frustrating for developers.


I'd rather have a couple of false positives and then selectively disable warnings (I think one can use pragmas to disable specific warnings for specific regions of code in gcc).

At least, I would like an additional flag that really does enable all warnings, say -Wabsolutelyall or something like that.


You probably don't want that, but a flag that enables all sensible messages, yet does not enable stuff that is there just for compatibility or because it's useful in some context.

It should also disable Werror, otherwise we will be claiming for some new flags in a few years.


that's a terrible excuse for not enabling them in -Wall. Is there a -Wreally-all?


Clang has -Weverything. GCC doesn't: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66293


It looks like -Weverything for gcc would make a lot of code not compile (along with -Werror). For example, it would include -Weffc++, right? Those are warnings if your code just doesn't obey one guy's now-outdated opinions of what C++ should look like.

That's what this says at the bottom:

https://github.com/Tuplanolla/gcc-weverything


> For example, it would include -Weffc++, right?

Maybe, but where's the problem ? Just add -Wno-effc++ afterwards.


Yes, -Weverything + -Werror is often a silly combination. That isn't a GCC-specific problem, though. Clang with both flags will fail to compile quite a lot of code, too.

As the other reply points out, you can just use -Wno-effc++ on top of -Weverything. You could also avoid that particular problem by not using C++ ;-).


Note that Clang recommends you don't use -Weverything because it contains warnings that are in development or have a known high false-positive rate.


The actual reason why -Wall doesn't include more warnings is because too many people rely on the fact that -Wall includes warnings for questionable code that is easy to avoid. It's a bad name, but it's not being changed any time soon.


Everybody should use this in their new projects: http://shitalshah.com/p/how-to-enable-and-use-gcc-strict-mod...


If you knowingly upgrade libraries and/or other project dependencies you wouldn't expect backward compatibility (assume semver conventions), do you?

Why would upgrading a compiler be any different?


I'll bite. It seems like a number of these would complain about perfectly benign code.

Here's an example I can think of:

    b = (a == 0) ? 42 : 42;
A lot of readers are probably thinking this line is absurd and why on earth would a compiler not yell at you and call you a bad human being for writing this?

But let's be creative ... What if each of those 42s are expansions of different macros that vary based on an ifdef? You could have one platform or configuration where it expands to ? 43 : 42, and another where it expands to ? 42 : 42. Since the preprocessor does those expansions early, your compiler doesn't know the difference at the time of generating the warning. I just confirmed this in a small program, replacing the first 42 with FOO, the 2nd with BAR, and #defining them both to 42:

    warn.c:9:23: warning: this condition has identical branches [-Wduplicated-branches]
        b = (a == 0) ? FOO : BAR;
Overall I would say items in this list are not equally bad. Some of them unambiguously you want to flag. Others are things that people might do intentionally.


This is another reason why macros should not be treated or even viewed as source transformation passes. There is information loss involved in that. There's no reason the compiler shouldn't be able to only warn in the absence of a macro expansion.


The compiler passes of both GCC and Clang are aware of macro expansions since they are able to tell you about errors which occur after a macro expansion, i.e. the error messages similar to "... in expansion of macro X ...".


Yeah I'm aware. I'm more referring to the comments here, where people use "this is generated by a macro" as a justification even though that only makes sense if you think of preprocessed code as a separate stage from the actual code, which I'm arguing you shouldn't. That said though, I'm surprised they don't treat it that way for other cases like these?


Sometimes you might care about differing macros that expand to the same thing in a case like this example. The key point is the compiler can't read your mind to know when those cases are.


In this case, it'd probably be best to have another ifdef that makes

    b = 42
If the two macros are expanded to the same value. The compiler should optimize it out, anyway, but I'm a pythonista and I believe explicit is better than implicit, so, if you can write the optimal code explicitly, you probably should (unless it becomes unreadable if you do).


Clang (since LLVM version 4.4) has flag -Weverything which really reports everything

https://www.bignerdranch.com/blog/a-bit-on-warnings/


clang doesn't document the full list of warning flags that it supports, but the clang source code has a list, including the hierarchy of meta flags that enable other flags:

https://github.com/llvm-mirror/clang/blob/master/include/cla...

http://fuckingclangwarnings.com/ has descriptions of many of the warnings, but more recent warnings aren't covered because the website hasn't been updated since 2014.



Thank you! I've never seen that page. It doesn't show up when searching for "clang warnings" in DuckDuckGo or Bing, but I see now that it is Google's result #2.

I just found a GitHub repo that lists which warning flags are supported by different versions of clang and gcc:

https://github.com/Barro/compiler-warnings


I found -Weverything seriously annoying. Declare a structure, and clang warns about padding bytes being added. Make it a packed structure, and clang warns about the lack of padding bytes. Useless! Okay, I can see the warning on packed structures, as that's usually a compiler extension, but a plain struct?

Seriously?


There is also -Wlogical-op-parentheses, which rather condescendingly implies that you don't know the precedence of || vs &&. (The main use of parentheses is specifically to indicate precedence different from the usual, since otherwise they just add noise to the code and reduce clarity.) I half-expect -Warithmetic-op-parentheses or even -Wop-parentheses to show up in some future version of clang...

Unfortunately there's no corresponding -Wuseless-parentheses, which if present would truly make -Weverything a "damned if you do, damned if you don't".


I think clang added a -Weverything just to prove a point why it's a bad idea to actually contain _all_ warnings in -Wall.

As you say, some warnings are contradictory and it's up to the project to decide which one of them is more important.


Then turn off that warning if you don't like/want it.


The point is there's a warning either way. Pack the structure, and you get warnings. Don't pack the structure, get warnings. To me, that's being needlessly pedantic.


I'm not convinced I want Scott Meyers yelling at me with -Weverything as part of -Weffc++. I think I side with the gcc devs here on not having a flag to enable every possible warning ever.


You can just use `-Weverything -Wno-effc++` then


Why not have the flag and not use it if you don't want it? That way other users of gcc who want that flag are not deprived of that feature.


Most of them sound like they might have a large amount of false positives if turned on for an existing code base (a "false positive" in this case would be a case where you would dismiss the warning after investigation). A large amount of warnings that don't point to "actual problems" causes people to ignore or overlook the actually important warnings.

(I'm not saying that these warnings are pointless, but you can have a bug-free code base that triggers a lot of these)


Can't you use pragmas to silence those specific cases?


> Why are those warnings not automatically included

People who do production builds with -Wall and -Werror ruin it for the rest of us.


Compilers should at least support -Wall2017 or something then, enabling warnings in existence in that year and failing compilation for years in the future.


Yes, this is exactly right. If you want to automatically upgrade a thing, you need a way for them to pin their existing config so it doesn't break. A good analogy here is Stripe's API versioning: they keep making it better, but you're pinned to the version you start with until you upgrade (which you can do selectively).


Ouch.

In OCaml there is a very simple rule: Development builds run with "-warn-error", which is OCaml-speak for "-Werror". Production builds run without that option - simply because future version of ocamlc might introduce new warnings that would make the build fail.

This makes a lot of sense to me.

Why did the C/C++ ecosystem adopt a different aprroach to production build compiler options?


There isn't really a single coherent "C/C++ ecosystem", but for your average "build with make" program there isn't really a 'production' vs 'development' build distinction that would be easy to hang this off. (QEMU enables -Werror for builds from a git tree but we had to put a bit of special purpose in our configure script to do that.) You can't tie it to "is this a debug build" because some warnings only appear for optimised builds so ideally you want to have the check for both debug and optimised.


You could add -Werror to cflags in the makefile on the condition that some release build specific environment variable is set, though just adding separate release/debug/dev targets doesn't seem too tricky.


It's a design goal to be flag-compatible with gcc, so in general all these warnings are or will be supported in clang.


> Why are those warnings not automatically included in "-Wall -Wextra"?

Not all of them are actually errors, maybe depending on your style. For example, shadowing a variable is a well-defined feature of the C language, and so some code patterns take advantage of that fact. Other people avoid shadowing like the plague.


IMO shadowing is wrong in the same function and needs to be punished :) but "shadowing" other scores like base classes or class members or the is definitely okay, since you should be using a qualifier like foo:: or this-> to access them anyway.


> you should be using a qualifier like foo:: or this-> to access them anyway.

I think a lot of C++ devs will disagree with this. It's also very uncommon from my experience.

One could argue that this should be solved by semantic highlighting or naming conventions instead.


Yeah, I would disagree with a lot of C++ devs on this. But I strongly stand by my opinion.

First of all, it's not even a substitute. m_foo won't work if it's declared in a class based on a template parameter; you'll still need a qualifier. And suddenly that makes the code inconsistent, which in itself is kind of bad. Inconsistency makes it harder to read code and spot mistakes. And why should how you refer to a variable change based on the mere fact that the class containing it takes a template parameter?

Second, there is really no good reason why you should have to change

  class C { C(int width) : value(width) { } int width; };
to

  class C { C(int width) : m_width(width) { } int m_width; };
Ambiguity? Not really. If you actually follow the convention of qualifying member variables with this-> or the like, nobody would ever get confused at whether the "x" in x = 1 refers to a local variable or to a member variable. Literally the only reason to do this is laziness in typing, and C++ was most definitely not designed to be the language for saving keystrokes. You just have to get that notion out of your system when using the language; it just leads to worse code (speaking from experience).

Furthermore, there is no reason to encode the member-ness of a variable into its name. What's so special about that information? Should we start encoding the access specifiers too (private/public/etc.)? What about whether it's a class member? s_m_priv_width instead of just width? Why can't we just call the variable whatever darn thing it represents?

And my tongue is halfway in my cheek here, but this feels akin to naming your American kid AmeriAlex instead of just Alex. What sense does that make?

Finally, let's get semantic highlighting out of the way -- it fundamentally cannot with C++ syntax or with the C preprocessor, and it's twice as impossible with the two. It's never clear what m_foo refers to. Is it a global or is it in the base class? If it's in a header file included by two other .c files, and the preceding header files declare different base classes, what color should it have? It would refer to different things (maybe in one case in a namespace, and in another case in a class). Coloring is just an unsolvable problem in this language; it's not reliable in telling you where a variable referred to is declared.


I agree with you, I also don't like these naming conventions. Still I don't use this-> and have never been bitten by a shadowed variable (yet).

> class C { C(int width) : value(width) { } int width; };

You mean to write

  class C { C(int width) : width(width) { } int width; };
here, right?


That's why they are warnings, not errors. You don't have to use -Werror...


I suspect macro expansion causes too many false positives for many of these.


They should allow disabling some of these for macro expansions then.


some have a lot of false positives (even some in -Wextra have had false positives due to bugs), so if you compile with -Werror (as you should do) it can be troublesome.


Because of -Werror.


My absolute favorite for modern C++ is

-Werror=switch

Using this, you can enforce that a switch statement over an enum value is complete (covers all cases) simply by omitting the default case.


You're not the first person I've seen suggest that this condition is worth flagging, but I don't completely understand why. People who conform to such a style then go on to add a "default" case that is a no-op, which I guess is more explicit, but also feels a little tedious and verbose. Switch statements in C can do all sorts of funky things (vide: Duff's device) so I think omitting a case label for an enum value feels like a much lesser crime. What if you just don't have meaningful things to do for that enum value?


> I think omitting a case label for an enum value feels like a much lesser crime.

This is not about the coding police coming to get you but preventing you from shooting yourself into your feet when you extend the enumeration. Quite often you want to cover all cases and write code that signals an error if the default branch is called. CL even has a special version of the CASE macro (CL's "switch") called ECASE[1] with the only difference being that ECASE signals an error on unhandled cases.

[1] http://www.lispworks.com/documentation/HyperSpec/Body/m_case...


> This is not about the coding police coming to get you but preventing you from shooting yourself into your feet when you extend the enumeration.

This.

And not necessarily yourself, but whomever maintains the software after you.

Of course this doesn't prevent someone from adding a "default" label in some future iteration.


I think you guys are missing something in my perplexed state at this construct. I'm not saying it out of lack of experience or because I disbelieve in the "going out of my way to make things more maintainable though verbosity" aspect. It's just that many times in practice I have rarely seen an honest switch statement that really needs case labels for all possible values. Much more common in my experience, 1 or more cases are no-ops. I think it's pointless to list out the no-ops, but I've seen people advocate just that.

There are exceptions to this "some cases are no-ops" thing. But I think it's the minority of switches I've seen.

From that perspective it does seem like the coding police angle. It's reminiscent of certain things in Java, like making a bunch of implicit conversions into errors, or C#, which forbids implicit fall-through on a switch. Those too are for safety and maintainability. They also make a lot of constructs more ugly. My question is: is it worth it? In this case (unintended pun) I don't think so.


Really? I've literally never written (or at least I don't ever remember writing) a case statement without either specifying every possible value or adding a meaningful default case.


I should have clarified more; you'd use this for switches where you need to always handle all cases. Then if a you add a new value to the enum but forget to update the switch, you get a compile-time error instead of a buggy program.


But the flag affects all switches on enums.


> What if you just don't have meaningful things to do for that enum value?

Then you use:

  default: break;
It's a small price to pay for the consistency checks elsewhere when you don't have a default.


Consider static type checking when adding a new value to the enum. If the switch has a default, then obviously you lose this, but it's nice for the others.


You're arguing that there are cases where the warning doesn't apply... well that's why it's just a warning not an error. You're not disallowing using switch in other ways - you're just warning yourself when you do.


Except that the comment I'm replying to says -Werror, meaning they would like to turn this warning into a build break. That seems a bit like language fascism to me. (And if it's not a build break, leaving the warning on stderr during a build is noise, which will cause you to ignore more important warnings.)


Warnings are good, but it's a pity there's no good warning suppression method which is compact (and does not require to disable the warning on the entire file).

People generally "fix" warnings by writing equally useless statements like "x = x" (common for -Wunused). I've seen codebases which aim for zero warnings, but with -Wextra/-Weverything it's just silly. Warnings are supposed to be just that: warnings.

I'd rather want a clean way to suppress the warning and mark it as such instead, so that's hidden by default when the programmer takes note of it.

We have push/pop pragmas, but they are even worse readability-wise. I'd rather have a neighboring comment with dedicated syntax instead.


You can enable/disable warnings in gcc on a line-by-line basis using pragmas: https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html#D...


I mentioned that. However, push/pop statements require a minimum of 3 lines of pragmas to suppress an unused statement. This doesn't work, you'll end up disabling warnings for entire functions.


Most warnings are just warnings. A warning like "implicit cast from pointer to integer" is a red flag (I got bit by this one). I think there are a few others that are always worth taking a look at.

I like your idea of marking warnings. Warnings should be treated like the messages from any static analysis tool. A lot of times they are worth at least looking at but you should be able to mark them as "not a problem" if you think you know better. That way you will always look at new warnings and not end up ignoring all warnings because there are too many that are irrelevant.


For -Wunused specifically, the correct idiom is "(void)x;". It's a no-op that marks x as having been used. Yes, in an ideal world you wouldn't have to do that, but sometimes you're overriding a virtual method and genuinely don't need to use one of the parameters.


I wonder why -Wnull-dereference is not default.


I would guess "because people complain when it gives warnings related to macros." That's almost always the answer, especially for C.


I wonder the same. Especially because this might be the same code that deletes null-checks as an 'optimization due to undefined behavior' as discussed here [1].

I recall a bug report with quite the flame war in it, but couldn't find it.

[1] https://kb.isc.org/article/AA-01167


I usually add these old standbys:

-Wimplicit-function-declaration: warn when a function is used without being declared (not needed in -std=c99 mode, but I write in c90). Catches functions not being declared in header files, which would result in broken type checking.

-Wmissing-prototypes: external function is defined without a previous prototype declaration. This catches functions that should be static: you fix it by making the function static, or else by prototyping it (in a header) so that it is properly public.

-Wstrict-prototypes: catches accidental old-style definitions or declarations, like void foo(), which isn't a prototype.


> Catches functions not being declared in header files, which would result in broken type checking.

Actually it could be much more interesting than that -- the implicit function decl will be `int foo(int)` so you risk your compiler pushing the wrong args on the stack.


I'm confused by -Wrestrict. Isn't the whole point of the restrict keyword aliasing like that? what's the point of restrict if there is no enforcement by default?


The restrict keyword is supposed to enable optimizations, not to increase safety. You promise you won't access the memory location through different pointers and the compiler throws some precautions overboard.


Honest question: Are these warnings smart enough to fire only against "raw" source code? Many of the first few I could see happening from macro expansion and it would be nice to just let those go without warning and let the optimizations hopefully pick up the pieces. Or is that some of the false positives these bring up that were referenced?


Unless you enable -Wsystem-headers, GCC doesn't warn about things in system headers, so for noisy headers it's easiest to mark them as such (with -isystem <dir>).


clang implements it that way, most (all?) warnings from macro expansion are omitted. Maybe gcc does the same for some warnings, but I've only noticed it for clang.

I find clang's behavior particularly dangerous for warnings referring to removal of null pointer checks, which are undefined behavior (if (this != nullptr)...)

There should perhaps be an option to control this.


I think one of the most amazing things I ever found out about clang is that you can change warning settings directly inside the file with special #pragma's, without having to modify anything in the build process itself.


Same as gcc.


I find more useful to make the program also warning free in clang instead of adding such options.


clang just adds more stuff to -Wextra but you can get most of the warnings in GCC too.


It's more than only warnings.

If you code runs fine with two or three different compilers you can also be sure that you don't rely on undefined or compiler specific behaviour.

It's the same reason some multiplatform libraries are extremely battle tested and comparatively bug free when compared to single platform code.


Easier would be to use AX_COMPILER_FLAGS from https://www.gnu.org/software/autoconf-archive/ax_compiler_fl...

This probes for:

    checking whether C compiler accepts -Werror=unknown-warning-option... yes
    checking whether C compiler accepts -Wno-suggest-attribute=format... yes
    checking whether C compiler accepts -fno-strict-aliasing... yes
    checking whether C compiler accepts -Wall... yes
    checking whether C compiler accepts -Wextra... yes
    checking whether C compiler accepts -Wundef... yes
    checking whether C compiler accepts -Wnested-externs... yes
    checking whether C compiler accepts -Wwrite-strings... yes
    checking whether C compiler accepts -Wpointer-arith... yes
    checking whether C compiler accepts -Wmissing-declarations... yes
    checking whether C compiler accepts -Wmissing-prototypes... yes
    checking whether C compiler accepts -Wstrict-prototypes... yes
    checking whether C compiler accepts -Wredundant-decls... yes
    checking whether C compiler accepts -Wno-unused-parameter... yes
    checking whether C compiler accepts -Wno-missing-field-initializers... yes
    checking whether C compiler accepts -Wdeclaration-after-statement... yes
    checking whether C compiler accepts -Wformat=2... yes
    checking whether C compiler accepts -Wold-style-definition... yes
    checking whether C compiler accepts -Wcast-align... yes
    checking whether C compiler accepts -Wformat-nonliteral... yes
    checking whether C compiler accepts -Wformat-security... yes
    checking whether C compiler accepts -Wsign-compare... yes
    checking whether C compiler accepts -Wstrict-aliasing... yes
    checking whether C compiler accepts -Wshadow... yes
    checking whether C compiler accepts -Winline... yes
    checking whether C compiler accepts -Wpacked... yes
    checking whether C compiler accepts -Wmissing-format-attribute... yes
    checking whether C compiler accepts -Wmissing-noreturn... yes
    checking whether C compiler accepts -Winit-self... yes
    checking whether C compiler accepts -Wredundant-decls... (cached) yes
    checking whether C compiler accepts -Wmissing-include-dirs... yes
    checking whether C compiler accepts -Wunused-but-set-variable... no
    checking whether C compiler accepts -Warray-bounds... yes
    checking whether C compiler accepts -Wimplicit-function-declaration... yes
    checking whether C compiler accepts -Wreturn-type... yes
    checking whether C compiler accepts -Wswitch-enum... yes
    checking whether C compiler accepts -Wswitch-default... yes
    checking whether C compiler accepts -Wno-suggest-attribute=format... no
    checking whether C compiler accepts -Wno-error=unused-parameter... yes
    checking whether C compiler accepts -Wno-error=missing-field-initializers... yes
    checking whether C compiler accepts -Werror=unknown-warning-option... (cached) yes
    checking whether the linker accepts -Wl,--no-as-needed... no
    checking whether the linker accepts -Wl,--fatal-warnings... no
    checking whether the linker accepts -Wl,-fatal_warnings... yes
    checking whether the linker accepts -Wl,-fatal_warnings... yes




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact

Search: