On 03/23/14 02:22, Jordan Justen wrote: > 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?
In my opinion: yes. I save build logs and I can grep them for warnings automatically, in my build script. I'd continue to rid my patches of warnings, for one GCC version, and two arches (X64 and Ia32). This is scriptable on my end and the build times wouldn't increase intolerably. > 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.) Importantly, it would not cause new warnings issued by other compilers to break the build for other developers and users. I don't mind cleaning up warnings that I only learn about from reports *if* I have more flexibility in scheduling the fix. "My patch breaks the build for some users" feels very differently from "my patch causes warnings for some users, which they probably don't notice". Naturally you'd have to trust contributors that they'll deal with the warnings *eventually*. > 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. Ideally this would be centrally addressed. I singled out OVMF / gcc because these seemed more approachable (less difficult to argue for than "BaseTools/Conf/tools_def.template") and this combination is what affects me directly. > I also think that the particular > warning you are considering masking does have value in some cases. For me personally the false positives outweigh the true positives (because I haven't seen one instance of the latter kind, and have several of the former). I do want to preserve the warnings themselves (I don't want to silence them), I only want to prevent them from breaking the build. > 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. Since you test-build the patches anyway before committing them, with a number of compilers, ie. you catch such warnings (that I can't) before committing anyway, how about doing those builds as part of your first review? Then the very first set of your review comments would contain such remarks as well, and I could address them together with the functional / design observations. Of course the next version of the patchset could theoretically introduce warnings again. I nonetheless think that in total this strategy (= report warnings as early as possible), combined with the warnings-are-not-errors one, would ease my work greatly. (OTOH I do realize it would make review harder for you, probably increasing the review latency.) > 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?) I didn't mention the git fiasco because I've been trying to achieve something -- I didn't want to become my own enemy by listing negative experiences from the past. But I most certainly thought of how the git discussion had gone off a cliff, and yes you are right, I have imagined this thread about warnings petering out just the same. Most people are unaffected, and Laszlo whines all the time anyway. (This is again why I singled out OVMF / gcc for square one.) 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-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
