On 03/22/16 04:21, Gao, Liming wrote: > Laszlo: > Could we separate the warning option topic?
Sure. > I think we still need > more discussion on how to enable warning unused-but-set-variable when > MDEPKG_NDEBUG is defined. One simple way is to disable this warning > when MDEPKG_NDEBUG is set. TBH, I shelved my patches after sending my last email -- since the warning cannot be generally enabled due to gcc not supporting LTO with edk2, I kind of lost interest. Thanks Laszlo > > For the patch to add UNUSED notation to Base.h, I have no comment. > > Thanks > Liming >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Saturday, March 19, 2016 12:22 AM >> To: Gao, Liming; Leif Lindholm >> Cc: Ard Biesheuvel; [email protected]; Andrew Fish; Kinney, Michael D; >> Qiu, Shumin >> Subject: Re: [edk2] [PATCH] MdePkg: add UNUSED notation to Base.h >> >> On 03/18/16 15:48, Gao, Liming wrote: >>> Laszlo: >>> >>> I understand the request to BaseTools is that AutoGen code can pass >>> GCC or MSFT compiler without any warning. If so, EDKII module can enable >>> non default warnings in itself. I will evaluate BaseTools. >> >> Thank you! >> >>> And, you raise one issue that the different ARCHs have the different >>> warning setting. I suggest to keep them consistent to avoid the >>> different build results. How about adding -Wno-unused-but-set-variable >>> option to AARCH64 DEBUG. >> >> Can we move in the other direction perhaps? Can we remove >> -Wno-unused-but-set-variable from the following settings: >> >> - GCC46_IA32_CC_FLAGS >> - GCC46_X64_CC_FLAGS >> >> (these are inherited by the >= 4.6 GCC settings for IA32 and X64,) >> >> - RELEASE_GCC46_ARM_CC_FLAGS >> - RELEASE_GCC47_ARM_CC_FLAGS >> - RELEASE_GCC47_AARCH64_CC_FLAGS >> - RELEASE_GCC48_ARM_CC_FLAGS >> - RELEASE_GCC48_AARCH64_CC_FLAGS >> - RELEASE_GCC49_ARM_CC_FLAGS >> - RELEASE_GCC49_AARCH64_CC_FLAGS >> >> I believe all of the above settings include >> "-Wno-unused-but-set-variable" only for the following reason: >> >> When GCC46 was enabled in BaseTools, the "-Wall" option (which we do >> enable as a basis) suddenly started including >> "-Wunused-but-set-variable" too. Namely, that warning option was new in >> the gcc-4.6 release, and "-Wall" by default included it: >> >> https://gcc.gnu.org/gcc-4.6/changes.html >> >> The edk2 tree wouldn't build with "-Wunused-but-set-variable"; so, in >> order to expedite GCC46 enablement, the "-Wno-unused-but-set-variable" >> was set. See commit 2bcc713e74b94. >> >> However, in reality, the number of warnings triggered by this setting is >> very low, in my testing -- just a handful. I listed them yesterday; the >> following modules are affected (while building ArmVirtPkg and OvmfPkg): >> >> - MdeModulePkg/Bus/Pci/PciHostBridgeDxe >> - UefiCpuPkg/Library/MtrrLib >> - UefiCpuPkg/PiSmmCpuDxeSmm >> >> If I send a series that cleans up these warnings, would you agree (and >> perhaps even test with your own platforms) to remove >> "-Wno-unused-but-set-variable"? >> >> I agree that we should unify the warning flags, but I think we should >> move towards stricter warnings, not laxer warnings -- provided we can >> adapt the tree more or less easily. >> >>> Besides, Shumin is a young man. J >> >> Thanks, I'll keep it in mind. :) >> >> Laszlo > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
