<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi David,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I think your guidance about disabling warnings for scenarios that Clang does not care to warn about is generally good.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:dblaikie@gmail.com]
<br>
<b>Sent:</b> Friday, March 27, 2015 10:32 AM<br>
<b>To:</b> LLVM Developers Mailing List<br>
<b>Cc:</b> Aaron Ballman; Chandler Carruth; Simon Atanasyan; Kaylor, Andrew<br>
<b>Subject:</b> Non-clang warning cleanliness in the LLVM project<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">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.<br>
<br>
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.<br>
<br>
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:<br>
<br>
template<typename T><br>
int func(T t) {<br>
return t.func() >> 8;<br>
}<br>
<br>
(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).<br>
<br>
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.<br>
<br>
How do people feel about working around this warning so we can keep it enabled?<br>
How do people feel about disabling this warning?<br>
How do people feel about disabling other non-Clang warnings which have false positives?<br>
<br>
(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)<br>
<br>
- David<o:p></o:p></p>
</div>
</div>
</body>
</html>