Re: [PATCH 4/4] BaseTools: resolve initialization order errors in VfrFormPkg.h

["Gao, Liming" <liming.gao@intel.com>]

We have verified this change with VS2015 compiler. There is no issue found. 

Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Gao, Liming
> Sent: Monday, December 11, 2017 10:22 PM
> To: Zenith432 <zenith432@users.sourceforge.net>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 4/4] BaseTools: resolve initialization order errors in VfrFormPkg.h
> 
> For VfrCompiler change, we will help verify VS tool chain. I think it is a good enhancement.
> 
> For UefiLib.h issue, I agree this is an issue. For short term, we disable this warning. For long term, I expect to find one compatible
> solution or the well discussed solution to reduce its impact. So, I ask the question on GCC and VS compiler behavior for BOOLEAN data
> type in function.
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Zenith432 [mailto:zenith432@users.sourceforge.net]
> > Sent: Monday, December 11, 2017 12:02 AM
> > To: Gao, Liming <liming.gao@intel.com>; edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH 4/4] BaseTools: resolve initialization order errors in VfrFormPkg.h
> >
> > On 10/12/2017 03:52 PM, Gao, Liming wrote:
> > > I think these patches resolves CLANG build issues in BaseTools. Do you verify them with GCC or VS tool chain?
> > >
> > GCC 7.2 does not give any of the warnings generated by clang while compiling BaseTools.
> > After applying the 4 BaseTools patches, it still does not give any warnings (or errors).
> > I don't have VS toolchain to try on.
> >
> > For VfrFormPkg.h, there is simpler solution to surround all the offending code in
> >
> > #pragma clang diagnostic push
> > #pragma clang diagnostic ignored "-Wuninitialized"
> > <offending code>
> > #pragma clang diagnostic pop
> >
> > I sent you this option last week, you said you prefer to resolve the reason for the "-Wuninitialized".
> > The reason is the attempt in 100 or so derived classes to force an initialization order different than C++.  It's
> > undefined behavior.  Any resolution for the undefined behavior involves changing those 100 derived classes, so a massive
> > code change.  The existing code works, it's just an attempt to get around standard C++ initialization order.  Clang
> > wants to give pedantic warnings, but can be silenced if the "undefined behavioral" code is known to work.
> >
> > The other 3 BaseTools patches are rather small.
> >
> > Also, with the VA_START(Args, BOOLEAN) bug (bug 741), there is an option of silencing clang locally around the offending
> > statement
> > #pragma clang diagnostic push
> > #pragma clang diagnostic ignored "-Wvarargs"
> > VA_START(Args, Iso639Language);
> > #pragma clang diagnostic pop
> >
> > And then remove global silencing of this warning in tools_def.txt.  That way any other not yet considered warnings will
> > be caught in the future.
> >
> > It's up to you how to go about it...  big code changes, silencing warning locally or silencing warning globally.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel