Our downstream project has a fairly strict security policy and recently we discovered that many MSVC warnings in LLVM project are suppressed.
At least some of them look concerning. For instance (non-exhaustive),
-wd4146 # Suppress 'unary minus operator applied to unsigned type, result still unsigned'
-wd4244 # Suppress ''argument' : conversion from 'type1' to 'type2', possible loss of data'
-wd4267 # Suppress ''var' : conversion from 'size_t' to 'type', possible loss of data'
-wd4291 # Suppress ''declaration' : no matching operator delete found; memory will not be freed if initialization throws an exception'
-wd4624 # Suppress ''derived class' : destructor could not be generated because a base class destructor is inaccessible'
-wd4722 # Suppress 'function' : destructor never returns, potential memory leak
-wd4245 # Suppress ''conversion' : conversion from 'type1' to 'type2', signed/unsigned mismatch'
-wd4389 # Suppress 'signed/unsigned mismatch'
Practically, any suppressed type mismatch may result in an UB. Other cases are subject to UB / ill-formed code as well, it feels like.
Is there a known explanation for why these are disabled?
Would it be possible to remove (some of) the suppressions to maintain cleaner code? (And is there a proper procedure to do this, assuming the change is rather large / affects everything under LLVM?)
We disabled a bunch of MSVC warnings because they were noisy: generating warnings for constructs that other compilers don’t warn about, and aren’t that dangerous. Most of these were added a long time ago, so it’s possible they’re worth revisiting.
The “other compilers don’t warn” part is important because most LLVM developers use Linux as their preferred environment, and Windows buildbots are unfortunately not very reliable, so any Windows-specific warnings are likely to cause issues for normal development. Ideally, if we’re going to turn on an MSVC warning, we also want to turn on an equivalent clang warning.
If you want to propose turning some warning on by default, and it doesn’t require significant changes to LLVM to fix (maybe less than 100 changed lines or something like that), you can just post a patch to fix the code and remove the warning suppression. Otherwise, you probably want to post a proposal here discussing the scope of the changes and a transition plan. (In some cases, we could replace global warning suppressions with local pragmas to suppress specific warnings.)
For signed/unsigned, I think it’s been discussed previously, but they show up in so many places that it would be hard to transition.
For unary minus, I think it shows up fairly frequently in bit-twiddling code, and the equivalents don’t really help clarity.
I don’t recall anything about the other warnings you listed.
At least some of these warnings should behave better on VS2019+ then they did on older versions when they were first disabled, but as @efriedma-quic said it needs to be reviewed on a case-by-case basis.
Also bear in mind that fewer people build with MSVC so many warnings creep in when they’ve only been build on gcc/clang, and even fewer of them will bother to commit fixes.
Its been a while since I last looked at them but IIRC re-enabling the shadow variable warnings exposed a number of interesting issues that only static analysis had picked up on.
-wd4456 # Suppress 'declaration of 'var' hides local variable'
-wd4457 # Suppress 'declaration of 'var' hides function parameter'
-wd4458 # Suppress 'declaration of 'var' hides class member'
-wd4459 # Suppress 'declaration of 'var' hides global declaration'
LLVM codebase is designed to be built without exception, that seems pure noise?
For the rest, we often rely on the equivalent warning provided by clang to keep the codebase clean: we don’t want to just “please” MSVC when it is pedantic and we find clang (and sometimes gcc) to be historically better at not providing false positive for the same warning.
MSVC may have evolved since, the main thing I’d look for is not to change the codebase just to silence false positives (this is why we disable warnings for specific compilers in general).