On Sep 15, 2014, at 6:45 PM, Jordan Justen <[email protected]> wrote:
> On Mon, Sep 15, 2014 at 5:38 PM, Scott Duplichan <[email protected]> wrote:
>> Jordan Justen [mailto:[email protected]] wrote:
>>
>> ]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?
>>
>> From my point of view there are a couple of disadvantages to this approach.
>> The patch I submitted is a permanent fix. The code rework method requires
>> on-going work. That is, even if code changes can make the build pass today,
>> future code rework will be required as the project evolves. That is because
>> few developers use the old Microsoft tools. They will submit code that builds
>> cleanly with newer tools. But eventually someone will build with the old
>> tools and encounter one of these problems. That will require new code rework.
>> A more fundamental objection to me is the idea of changing working, portable,
>> bug-free code to work around the 'broken warnings' of old build tools. If
>> newer build tools didn't exist, then working around these warning limitations
>> might make sense. But newer build tools do exist, and in fact most developers
>> use them. I won't object further if someone else wants to change the code
>> for this reason, but I don't want my name on such a change.
>
> This would be a major change to how EDK II addresses these kinds of issues.
>
> EDK II (and EDK before that) has always pushed for a high number of
> warnings and caused a build errors on warnings.
>
> This strategy goes way back. I don't even know when it started. Maybe
> Vincent or Andrew know.
>
Well the problem with picking a compiler is you end up coding to the compiler
an not the pedantic C specification. So the idea behind supporting more
compilers is the clean up the C code. So warning == Error is good and more
compilers is better. I don’t think there is an argument here.
I would say we should keep the code clean with the latest versions of the
different compiler families we support. If this breaks older versions of the
supported compilers, especially with false positives then we should suppress
those false positive warnings for the old compilers. We need to assume the
newer compilers are doing a better job. If we hit the point that an old
compiler no long works it should not be listed in the
https://svn.code.sf.net/p/edk2/code/trunk/edk2/BaseTools/Conf/tools_def.template.
It seems like adding a newer version of the compiler to the tools_def.tempalte
file moves the bar forward. Again the argument for supporting more compilers
was to have more correct C code. Putting code in to work around false positives
is not that helpful, and it is probably OK to turn off warning in the old
compilers. But the old compilers should work or not be supported.
OK so maybe it is a little more complicated if you have a big group of
customers on some middle version of the compiler. like VS2008. So maybe you
decide to only suppress warnings on older compilers. But this should be a call
for the maintainer of that toolchain. I maintain Xcode/clang, but usually the
warnings just get better, and there is not a big issue with false positives. So
I would propose for every toolchain we identify the minimum version that has to
work, and older versions we are willing to suppress warnings. That way we keep
old stuff alive, but get the benefit of newer compilers, but don’t leave behind
a large group of folks that are using a slightly older compiler, and force a
bunch of code changes to deal with false positives.
Thanks,
Andrew Fish
> At any rate, it does indeed cause an ongoing drag, but changing this
> has never been seriously considered. Maybe this is as much due to
> momentum as anything. :)
>
> But, I don't think you've made the case that disabling these warnings
> will end up producing better code. (Personally, I couldn't say one way
> or the other.)
>
------------------------------------------------------------------------------
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
