David Blaikie via llvm-dev
2021-Jan-12 17:36 UTC
[llvm-dev] Miscellaneous warnings in code using Visual Studio
On Tue, Jan 12, 2021 at 9:34 AM Aaron Ballman <aaron at aaronballman.com> wrote:> On Tue, Jan 12, 2021 at 12:17 PM David Blaikie via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > > > If they prove to be good cleanup/improvements to the code in general - > but we don't try too hard to be -Werror clean on every compiler, mostly > just self-hosted clang. > > This doesn't match my understanding -- we have a documented CMake > option that controls -Werror and I think we've always tried to be > -Werror clean with Clang, GCC, and MSVC builds before to support that > build configuration. >Certainly I don't think we've ever outright rejected any path forward for avoiding a warning from any supported compiler (either by changing the code or suppressing the warning in the LLVM build in general) - though I don't think practically speaking we try very hard to keep all the supported compiler versions (not just brands, but the full supported version range of each) -Werror clean - about the cleanest would be a selfhost build, there are many buildbots/etc that check that is -Werror clean.> > > The line is fuzzy - if a warning isn't /too/ bad (ie: doesn't require > extreme contortions to the code to address the warning) then fixing > instances seems ok. If it's unhelpful (eg: a warning has a high false > positive rate and/or clang's equivalent warning is more nuanced and avoids > the need to touch false positives) we may disable warnings in LLVM. (this > doesn't address the issue of warnings in LLVM's public headers - not every > downstream consumer of LLVM is going to disable the same set of warnings, > so sometimes it's necessary to do some warning cleanup even for undesirable > warnings) > > I agree that we don't want large amounts of code churn for warnings > that are typically off by default in our default cmake configuration > (or could be made off by default if they're sufficiently low quality), > but I think this situation is different. I think these new warnings > were recently introduced and are ones we would typically fix with an > NFC patch. > > ~Aaron > > > > > On Tue, Jan 12, 2021 at 9:14 AM Paul C. Anagnostopoulos via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> > >> When I build on Windows 7 with Visual Studio, I get various warnings > that were ignored until I recently set > >> > >> LLVM_ENABLE_WERROR:BOOL=ON > >> > >> One example is shown below. Should I take the time to hunt these down > and fix them? > >> > >> > >> ********************************************************************** > >> ** Visual Studio 2019 Developer Command Prompt v16.8.1 > >> ** Copyright (c) 2020 Microsoft Corporation > >> ********************************************************************** > >> > >> D:\LLVM\build>build-targets.bat > >> 1 file(s) copied. > >> [363/1578] Building CXX object > lib\CodeGen\...LLVMGlobalISel.dir\MachineIRBuilder.cpp.obj > >> FAILED: > lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/MachineIRBuilder.cpp.obj > >> > C:\PROGRA~2\MIB055~1\2019\COMMUN~1\VC\Tools\MSVC\1428~1.293\bin\Hostx86\x86\cl.exe > /nolog > >> o /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE > -D_CRT_NONSTDC_NO_WARNINGS > >> -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS > -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTI > >> ONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE > -D_SCL_SECURE_NO_WARNINGS -D_UNICODE > >> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS > -Ilib\CodeGen\Global > >> ISel -IC:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel -Iinclude > -IC:\LLVM\llvm-project\l > >> lvm\include /DWIN32 /D_WINDOWS /WX /Zc:inline /Zc:__cplusplus > /Zc:strictStrings /Oi /Zc: > >> rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 > -wd4456 -wd4457 -wd > >> 4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 > -wd4610 -wd4510 -wd47 > >> 02 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 > -wd4204 -wd4577 -wd4091 > >> -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od > /RTC1 /EHs-c- /GR- -st > >> d:c++14 /showIncludes > /Folib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\MachineIRBui > >> lder.cpp.obj > /Fdlib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LLVMGlobalISel.pdb > /F > >> S -c > C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp > >> > C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp(657): > error C2220: t > >> he following warning is treated as an error > >> > C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp(657): > warning C4018: > >> '>=': signed/unsigned mismatch > >> [366/1578] Building CXX object > lib\CodeGen\...CMakeFiles\LLVMGlobalISel.dir\Utils.cpp.obj > >> ninja: build stopped: subcommand failed. > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> llvm-dev at lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210112/022a8e60/attachment.html>
Aaron Ballman via llvm-dev
2021-Jan-12 17:42 UTC
[llvm-dev] Miscellaneous warnings in code using Visual Studio
On Tue, Jan 12, 2021 at 12:37 PM David Blaikie <dblaikie at gmail.com> wrote:> > > > On Tue, Jan 12, 2021 at 9:34 AM Aaron Ballman <aaron at aaronballman.com> wrote: >> >> On Tue, Jan 12, 2021 at 12:17 PM David Blaikie via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >> > >> > If they prove to be good cleanup/improvements to the code in general - but we don't try too hard to be -Werror clean on every compiler, mostly just self-hosted clang. >> >> This doesn't match my understanding -- we have a documented CMake >> option that controls -Werror and I think we've always tried to be >> -Werror clean with Clang, GCC, and MSVC builds before to support that >> build configuration. > > > Certainly I don't think we've ever outright rejected any path forward for avoiding a warning from any supported compiler (either by changing the code or suppressing the warning in the LLVM build in general) - though I don't think practically speaking we try very hard to keep all the supported compiler versions (not just brands, but the full supported version range of each) -Werror clean - about the cleanest would be a selfhost build, there are many buildbots/etc that check that is -Werror clean.My recollection is that we only require the selfhost build to be -Werror clean (with a bot configured to test that) but that we strive to be -Werror clean when it won't introduce unwanted changes to the code (in which case, we're happier to disable the warning within CMake). So I think we may be saying the same things with different words, but maybe we have a different definition of "try very hard" here. :-) ~Aaron> >> >> >> > The line is fuzzy - if a warning isn't /too/ bad (ie: doesn't require extreme contortions to the code to address the warning) then fixing instances seems ok. If it's unhelpful (eg: a warning has a high false positive rate and/or clang's equivalent warning is more nuanced and avoids the need to touch false positives) we may disable warnings in LLVM. (this doesn't address the issue of warnings in LLVM's public headers - not every downstream consumer of LLVM is going to disable the same set of warnings, so sometimes it's necessary to do some warning cleanup even for undesirable warnings) >> >> I agree that we don't want large amounts of code churn for warnings >> that are typically off by default in our default cmake configuration >> (or could be made off by default if they're sufficiently low quality), >> but I think this situation is different. I think these new warnings >> were recently introduced and are ones we would typically fix with an >> NFC patch. >> >> ~Aaron >> >> > >> > On Tue, Jan 12, 2021 at 9:14 AM Paul C. Anagnostopoulos via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> >> >> When I build on Windows 7 with Visual Studio, I get various warnings that were ignored until I recently set >> >> >> >> LLVM_ENABLE_WERROR:BOOL=ON >> >> >> >> One example is shown below. Should I take the time to hunt these down and fix them? >> >> >> >> >> >> ********************************************************************** >> >> ** Visual Studio 2019 Developer Command Prompt v16.8.1 >> >> ** Copyright (c) 2020 Microsoft Corporation >> >> ********************************************************************** >> >> >> >> D:\LLVM\build>build-targets.bat >> >> 1 file(s) copied. >> >> [363/1578] Building CXX object lib\CodeGen\...LLVMGlobalISel.dir\MachineIRBuilder.cpp.obj >> >> FAILED: lib/CodeGen/GlobalISel/CMakeFiles/LLVMGlobalISel.dir/MachineIRBuilder.cpp.obj >> >> C:\PROGRA~2\MIB055~1\2019\COMMUN~1\VC\Tools\MSVC\1428~1.293\bin\Hostx86\x86\cl.exe /nolog >> >> o /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS >> >> -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTI >> >> ONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE >> >> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\CodeGen\Global >> >> ISel -IC:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel -Iinclude -IC:\LLVM\llvm-project\l >> >> lvm\include /DWIN32 /D_WINDOWS /WX /Zc:inline /Zc:__cplusplus /Zc:strictStrings /Oi /Zc: >> >> rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd >> >> 4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd47 >> >> 02 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 >> >> -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od /RTC1 /EHs-c- /GR- -st >> >> d:c++14 /showIncludes /Folib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\MachineIRBui >> >> lder.cpp.obj /Fdlib\CodeGen\GlobalISel\CMakeFiles\LLVMGlobalISel.dir\LLVMGlobalISel.pdb /F >> >> S -c C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp >> >> C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp(657): error C2220: t >> >> he following warning is treated as an error >> >> C:\LLVM\llvm-project\llvm\lib\CodeGen\GlobalISel\MachineIRBuilder.cpp(657): warning C4018: >> >> '>=': signed/unsigned mismatch >> >> [366/1578] Building CXX object lib\CodeGen\...CMakeFiles\LLVMGlobalISel.dir\Utils.cpp.obj >> >> ninja: build stopped: subcommand failed. >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> llvm-dev at lists.llvm.org >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > llvm-dev at lists.llvm.org >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev