How we made compiler warnings fatal in Firefox

Compiler warnings are mostly good: they identify real problems, and when false positives do occur they are usually easy to work around. However, if they’re not fatal, they tend to be ignored and build up. (See bug 187528 for an idea!)

One way to prevent the build-up is to make them fatal, so they become errors. But won’t that cause problems? Not if you’re careful. Here’s how we did it for Firefox.

  • Choose with some care which warnings end up fatal. Don’t be afraid to modify your choices as time goes on.
  • Introduce a mechanism for enabling fatal warnings on a per-directory basis. Mozilla’s custom build system used to have a directive called FAIL_ON_WARNINGS for this purpose.
  • Set things up so that fatal warnings are off by default, but enabled on continuous integration (CI). This means the primary coverage is via CI. You don’t want fatal warnings on by default because it causes problems for developers who use non-standard compilers (e.g. pre-release versions with new warning classes). Developers using the same compilers as CI can turn it on locally if they want without problem.
  • Allow per-file exceptions for particular kinds of warnings, because there are occasionally warnings you just want to ignore.
  • Fix warnings one directory at a time, and turn on fatal warnings for that directory as soon as it’s warning-free.
  • Invert the sense of the per-directory mechanism once you’ve converted more than half of the directories. For Mozilla code we now have the ALLOW_COMPILER_WARNINGS directive. It’s almost exclusively used for directories containing third-party code which is not under our control.
  • Gradually expand the coverage of which compilers you have fatal warnings for. Mozilla code now does this for GCC, clang, and MSVC.
  • Congratulations! You now have fatal warnings on everywhere that is practical.

With a setup like this, it’s possible for a patch to compile on a developer’s machine but fail to compile on CI. But that’s just one of many ways in which full CI runs may fail when local runs don’t. So it’s not as bad as it seems.

Also, before upgrading the compilers on CI you need to address any new warnings, by fixing them or suppressing them or filing a compiler bug report. But this isn’t a bad thing.

It took a long time for Firefox to reach this stage, but I think it was worth the effort. Thank you to Chris Peterson and all the others who helped with this.

2 Responses to How we made compiler warnings fatal in Firefox

  1. I have some CI running building daily Firefox with gcc & clang trunks with the option –enable-warnings-as-errors. Hopefully, this will help addressing the issue early in the future.

    By the way, the end of the builds looks great now:
    342 compiler warnings present.
    warning: obj-x86_64-pc-linux-gnu/dist/include/mozilla/CountingAllocatorBase.h:125:5 [-Wundefined-var-template] instantiation of variable ‘mozilla::CountingAllocatorBase::sAmount’ required here, but no definition is available
    (suppressed 3 warnings in db/sqlite3/src)
    (suppressed 4 warnings in gfx/cairo)
    (suppressed 1 warnings in intl/hyphenation/hyphen)
    (suppressed 9 warnings in intl/icu)
    (suppressed 1 warnings in ipc/chromium)
    (suppressed 7 warnings in js/src/ctypes/libffi)
    (suppressed 7 warnings in media/ffvpx)
    (suppressed 1 warnings in media/libmkv)
    (suppressed 4 warnings in media/libsoundtouch)
    (suppressed 1 warnings in media/libspeex_resampler)
    (suppressed 31 warnings in media/libtheora)
    (suppressed 62 warnings in media/mtransport/third_party)
    (suppressed 24 warnings in media/webrtc/trunk)
    (suppressed 2 warnings in modules/woff2)
    (suppressed 20 warnings in netwerk/sctp/src)
    (suppressed 32 warnings in nsprpub)
    (suppressed 118 warnings in security/nss)
    (suppressed 4 warnings in third_party/aom)
    (suppressed 2 warnings in toolkit/crashreporter/google-breakpad)

  2. Compiler warning counts are now tracked in Perfherder’s “build_metrics” data, so you can watch the trends since May:

    https://is.gd/RkRgoR