[LLVMdev] Non-clang warning cleanliness in the LLVM project

[Kaylor, Andrew]

Hi David,

I think your guidance about disabling warnings for scenarios that Clang does not care to warn about is generally good.

The gray area, to my way of thinking, is warnings that in the general case Clang does report but which have special cases that are reported by other compilers and not by Clang.  We saw this with the warning about local variables that were initialized but never referenced.  I wouldn’t want to disable that warning across the board for MSVC because it is a useful warning.  In that particular case, I think it would be useful for Clang to also report the case where an object variable is only ever used to call static functions in its class.  The case where specific template instantiations could cause the warning is more problematic, and sounds like the shift warning you describe below.

My idealistic preference would be to leave these gray-area warnings enabled.  In the case of the initialized-but-not used warnings I just mentioned, I feel like all of the changes I made to avoid the warnings resulted in objectively better code (though that wasn’t the case with my initial attempt to fix one of them).  On the other hand, I think it’s likely that cases will arise from time-to-time where a non-clang warning can’t be corrected with a change that we would want to make.  My personal preference would be to insert a compiler-specific pragma to suppress these warnings on a case-by-case basis.  I don’t think it will be common.

Of course, this won’t stick if we don’t have buildbots that are looking for clean compiles with non-clang compilers.  I don’t know what the state of GCC buildbots is in this respect.  Obviously what few MSVC builders we have aren’t doing it yet, but I’d like to at least enable the “all warnings” option by default for Windows very soon.

-Andy


From: David Blaikie [mailto:dblaikie at gmail.com]
Sent: Friday, March 27, 2015 10:32 AM
To: LLVM Developers Mailing List
Cc: Aaron Ballman; Chandler Carruth; Simon Atanasyan; Kaylor, Andrew
Subject: Non-clang warning cleanliness in the LLVM project

So a while back we took a moderately aggressive stance on disabling GCC warnings with false positives, specifically those related to uninitialized variables. In cases where GCC suggested initializing a variable yet the algorithm was safely initializing the variable, adding the GCC-suggested initialization could thwart tools like MSan/ASan. So it was suggested that we should not abide by GCC's warning and rely on Clang's more carefully limited warning without the problematic false positives.

Recently Andy Kaylor's been working on getting the MSVC build warning clean by a combination of disabling warnings and fixing a few cases of relatively low frequency.

I've generally been encouraging people to aggressively disable non-clang warnings whenever there's a false positive (anything other than a bug, or at least anything that doesn't strictly improve readability) and one such instance of this is in the review for r233088 which amounts to something like:

template<typename T>
int func(T t) {
  return t.func() >> 8;
}

(it's more complicated than that, but this is the crux of it) - for some instantiations of this template, T::func returns an 8-bit type, and MSVC warns that the shift will produce a zero value. The code is correct (the author intended this behavior, because some instantiations produce a wider-than-8-bit- type and the higher bits are desired, if present).

I suggested disabling this warning, but that would mean we miss out on the true positives as well (Clang doesn't seem to have any warning like this), though we haven't seen these very often.

How do people feel about working around this warning so we can keep it enabled?
How do people feel about disabling this warning?
How do people feel about disabling other non-Clang warnings which have false positives?

(in the case of Clang warnings with false positives we should take the same approach, except we also have the option to improve the quality of the warning by fixing Clang, which is what we generally try to do)

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150327/aeaa261c/attachment.html>