I think in most cases our EDK II code base could compile cleanly with these toolchains if minor changes were made to the source. For example, I sent a patch in February: "OvmfPkg: Fix VS2005 compiler warnings" and, you actually commented on the thread.
Anyway, I don't work in windows too often, and I ended up not finishing cleaning up that patch and getting it upstream. But, generally, I think this is the approach that we prefer to dealing with compiler warnings. Can we clean up warnings in these packages through code rework instead of disabling warnings? Of course, if a warning impacts a huge portion of the code base, and clearly has no value, then it might be disabled. For Microsoft, we seem to disable warnings this in ProcessorBind.h: MdePkg/Include/Ia32/ProcessorBind.h (Rather than on the compiler command line.) Regarding the subjects on your patch emails, I think many of these should look like "BaseTools: ..." since they are modifying BaseTools. Patch 2 seems interesting, since as you point out it makes VS2013 match the behavior of older VS toolchains. I don't know if IA32 builds get to assume SSE2 support or not. I appreciate that you've tried to emulate the patch numbering of git format-patch and git send-email. In case you are interested, I noticed two differences: 1. git format-patch inlines the patch by default This is nice since you can easily reply and 'quote' the code during code review. 2. git send-email uses the email In-reply-to header to thread subsequent patch postings together This is nice if you use an email program that can show the email thread (and thus, the patch series) as one group. -Jordan On Sun, Sep 14, 2014 at 9:03 PM, Scott Duplichan <[email protected]> wrote: > Fix build failures for Microsoft tool chains. Tool chains tested: > DDK3790, VS2003, VS2005, VS2008, VS2010, VS2012 and VS2013. The > build.exe command lines tested are attached. The bulk of the build > failures are due to warnings produced by older tool chains but not > by newer tool chains. The following C code illustrates how the newer > tool chains have been improved to recognize and eliminate integer > truncation warnings in cases where no integer truncation occurs. > The code also shows that the gcc optional -Wconversion warning is > similar to, but not identical, to its Microsoft counterpart when > newer gcc releases are used. > > #if defined (__GNUC__) > #include <stdint.h> > #else > #define uint8_t unsigned __int8 > #define uint16_t unsigned __int16 > #define uint32_t unsigned __int32 > #define uint64_t unsigned __int64 > #endif > uint8_t test1 (int x) {return !x;} > uint8_t test2 (int x) {return x == 1;} > uint8_t test3 (int x) {return x && 1;} > uint32_t test4 (uint64_t x) {return x >> 32;} > uint32_t test5 (uint64_t x) {return x & 0xFFFFFFFF;} > enum {a=1, b=2}; uint16_t test6 (int x) {return x ? a : b;} > uint8_t test7 (uint8_t a, uint8_t b) {return a + b;} > /* > W A R N I N G C O U N T S > Tool chain test1 test2 test3 test4 test5 test6 test7 > DDK3790 /W4 1 1 1 1 1 1 0 > VS2003 /W4 1 1 1 1 1 1 0 > VS2005 /W4 1 1 1 1 1 0 0 > VS2008 /W4 0 0 0 0 0 0 0 > VS2010 /W4 0 0 0 0 0 0 0 > VS2012 /W4 0 0 0 0 0 0 0 > VS2013 /W4 0 0 0 0 0 0 0 > gcc430 -Wconversion 1 1 1 1 1 1 1 > gcc490 -Wconversion 0 0 0 1 0 0 1 > */ > > This patch set disables warnings in older tool chains when they > are not compatible with newer tool chains. One reason is practicality: > fewer developers use or even have access to the older tool chains. > Another reason is that the warnings produced _only_ by the older tool > chains are generally not useful. Disabling such warnings from older > tool chains means that a developer who uses the older tool chain might > submit code that will have warnings when compiled with a newer tool chain. > This problem will be caught quickly because more users use new tool chains. > If instead it was decided that older tool chains must build warning free, > then developers using new tool chains would have to try to learn about the > quirks in the older tool chains. > > One interesting warning case disabled by this patch set is VS2013 warning > C4701/C4703 (potentially uninitialized local variable used). In a few cases > this warning is valid and useful. It sometimes finds cases where ignoring the > return code from a function that initializes a pointer leaves the pointer > uninitialized. But there are a some problems with this warning. One is that > the warning is incorrect in many cases. Another is that few developers have > access to VS2013. > > There is no perfect solution to the current problem of multiple tool chains. > A long term solution is to get a gcc build working smoothly under Windows. > Then choose a particular version of gcc as the official tool chain. Code > must compile warning free with the official tool chain. Gcc is free and > available for Windows, Linux, Apple, etc. So anyone could use it. This > is the approach used by the coreboot project. > > Thanks, > Scott > ------------------------------------------------------------------------------ Want excitement? Manually upgrade your production database. When you want reliability, choose Perforce Perforce version control. Predictably reliable. http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
