Batu69 Posted July 5, 2017 Share Posted July 5, 2017 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 code used to have a moz.build 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 fix any new warnings. 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. Article source Link to comment Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.