For as long as I've been involved with edk2 (and Edk before that), there has always been a strategy of: 1. Ratchet up warnings as much as is tolerable 2. Make warnings cause a build error
I assume the goal is to try to keep the code base clean (for some definition of clean). I generally agree that it seems to force cleaner code. (Or is this a case of Stockholm syndrome? :) It has always added some maintenance burden. I think originally the burden was fairly minimal, because there was probably only 2 versions of MSVC and maybe 1 version of ICC supported. With 6 versions of MSVC and 6 versions of GCC, plus several other toolchains (and architectures), it does seem clear that the maintenance burden is higher now. Does the cost now outweigh the benefit of forcing a cleaner code base? Should we consider allowing warnings to just be warnings by default? Cleaning up warnings could then be a separate task which could be more focused. (Rather than incurring the extra context-switch burden of handling this for each and every new warning.) Or, as you suggest, we can consider something smaller and just tweak the set of warnings emitted per toolchain. Maybe this is enough, but there is also the chance that it will just the redraw the line of where warning based breakage occurs. I also think that the particular warning you are considering masking does have value in some cases. With regards to OVMF, I think I am not being fair to contributions when I test them against the toolchains that I have, and then feedback warnings based failures as review before committing a contribution. I like to try to get each patch in a series right (for some definition of right), but with regards to warnings based build breakage, this is too much to ask for from contributions. Anyway, I hope we do get more discussion on this topic and it doesn't just end with the usual sputtering out and inaction. (git anyone?) -Jordan On Fri, Mar 21, 2014 at 9:53 AM, Laszlo Ersek <[email protected]> wrote: > Hi, > > I'm interested in feedback for the following BuildTools-related idea. > > Over the past months working with OVMF I have distilled the following > conviction about invalid compiler warnings (false positives emitted for > intended and valid C constructs): > > (1) gcc likes to warn about uninitialized local variables that it finds > might be read without an intervening assignment. > > (2) MSVC like to warn about implicit type conversions in assignments and > argument passing. > > Experience has shown that, for OVMF code specifically, these warnings > are *always* wrong. They force us to mangle the code in order to shut > them up, and they tend to throw a wrench in the gears of development > process. > > The explanation for the latter fact is that the set of constructs that > the compilers warn about is a moving target. It varies with compiler > version *and* with target architecture (Ia32 vs. X64). > > In effect, requiring that these two classes of warnings be treated as > errors under all circumstances *guarantees* that our *valid* code will > be unjustifiedly rejected by *some* (compiler, architecture) > combination. We cannot prepare ourselves in advance for all possible > (compiler, architecture) pairs, hence the horrible disruption of > workflow that I mention above. > > The most recent example is: > > for Ia32 for X64 > --------- ----------------------- > gcc-4.4 breaks[1] builds without a squeak > gcc-4.8 breaks[2] builds without a squeak > > Note that [1] and [2] are *completely distinct* warnings. As in, when > building for Ia32, gcc-4.4 and gcc-4.8 warn about uninitialized > variables in *distinct* source files. The intersection of the warnings > emitted is the empty set. (I know this because I completed both builds > afterwards.) > > My proposal is the following. > > (1) For GCC, introduce the following additional compiler flags: > > GCC:*_GCC44_*_CC_FLAGS = -Wno-error=uninitialized > GCC:*_GCC45_*_CC_FLAGS = -Wno-error=uninitialized > GCC:*_GCC46_*_CC_FLAGS = -Wno-error=uninitialized > GCC:*_GCC47_*_CC_FLAGS = -Wno-error=maybe-uninitialized > GCC:*_GCC48_*_CC_FLAGS = -Wno-error=maybe-uninitialized > > Gcc-4.4 to gcc-4.6 cannot distinguish the following two cases: > - "I can *prove* that this local variable is read while uninitialized" > - "I can see some way, a possibly unreachable way, to read an > uninitialized local variable". > > Hence for gcc-4.4 to gcc-4.6, we should degrade -Wuninitialized from > error to warning. > > http://gcc.gnu.org/onlinedocs/gcc-4.4.7/gcc/Warning-Options.html#Warning-Options > http://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc/Warning-Options.html#Warning-Options > http://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Warning-Options.html#Warning-Options > > Gcc-4.7 and later *can* distinguish the above two cases (proven lack of > initialization vs. possible). Hence we should degrade only > -Wmaybe-uninitialized. > > http://gcc.gnu.org/onlinedocs/gcc-4.7.3/gcc/Warning-Options.html#Warning-Options > http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/Warning-Options.html#Warning-Options > > (2) For MSVC, come up with a similar solution that degrades errors about > *valid* implicit type conversions to warnings. > > Again, experience shows that, for OVMF specifically, treating these > warnings as errors produces *net negative* value. There has not been > *one* instance where the warning was right. > > The warnings should stay, but they must not stop the build. We will > continue to address warnings *that we see* while developing code. We > shall not allow the build to break for users who happen to use compilers > / arches different from ours. > > Thanks, > Laszlo > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/13534_NeoTech > _______________________________________________ > edk2-buildtools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel ------------------------------------------------------------------------------ Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ edk2-buildtools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel
