<div dir="ltr"><div>Should this have a -W flag?  Should it be on or off by default?<br></div><div><br></div><div>If we're not sure about how valuable this warning is, then we need a flag to turn it off.  Then, there is a maintenance cost associated with adding more -W flags.<br>
</div><div><br></div>The kinds of time wasters I used to hit are probably covered by -Wconversion and -Wsign-compare, but I never turned them on.<div style><br></div><div style>If people agree that there is value to having a mode that tries (best effort, not guaranteed) to warn when MSVC would, what about adding a flag that groups these together?  Like -Wmsvc-[123].  Then if we implemented a cl.exe compatible driver, we'd just alias /W3 to it.  And people could use clang without breaking 'cl /W[123] /Wx' builds.</div>
<div style><br></div><div style>Then this switch warning and others like it could go under -Wmsvc-3, without leading to a proliferation of -Wreally-obscure-warning flags.</div><div style><br></div><div style>I don't think you'd want to take this as far as maintaining a mapping of numbered MSVC warnings to clang warnings.  If someone is using 'cl.exe /W3 /wd...' they can come up with an equivalent expression in clang flags.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 3, 2013 at 11:23 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ping?<br>
<br>
I've given this some more thought, and I feel like Reid and Daniel (in<br>
the PR) hit the nail on the head: it is really nice to be able to<br>
write code in clang and not have to worry that I've introduced a<br>
warning in MSVC, especially since I try to compile everything with<br>
warnings as errors.  I understand the fear of trying to match<br>
diagnostics with other compilers, but where's the harm if it's not<br>
overly chatty and it can catch bugs?<br>
<br>
Also, I think the warning adds value over the empty statement warning<br>
because the situations are different.  Eg)<br>
<br>
switch (expr);  // Should warn because the ; is suspicious<br>
{<br>
  /* stuff */<br>
}<br>
<br>
// vs<br>
<br>
switch (expr) {<br>
  /* ToDo: finish this */<br>
}  // Should warn because the emptiness of the block is suspicious<br>
<br>
<br>
In the case where the user really does want an empty switch statement,<br>
they can selectively turn the diagnostic off.  This really shouldn't<br>
be a common case, after all.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Wed, May 29, 2013 at 4:10 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br>
> On Wed, May 29, 2013 at 3:49 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> wrote:<br>
>><br>
>> On Wed, May 29, 2013 at 7:35 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> wrote:<br>
>>><br>
>>> -Wunreachable-code doesn't warn on empty statement bodies for the<br>
>>> switch.  So, for instance:<br>
>>><br>
>>> int main() {<br>
>>>   int i = 0;<br>
>>>   switch(i) {}<br>
>>> }<br>
>>><br>
>>> yields no warnings in clang with -Wunreachable-code (or -Wall), but at<br>
>>> /W3 in MSVC you get: warning C4060: switch statement contains no<br>
>>> 'case' or 'default' labels.  Will it catch lots of bugs?  Probably<br>
>>> not.  But it also should have basically no false positives either.  It<br>
>>> would be nice for people who want their code to also be warning-free<br>
>>> under MSVC.<br>
>><br>
>><br>
>> I think we need more justification for adding the warning than this.<br>
>><br>
>> It doesn't seem reasonable to expect us to implement the union of all<br>
>> warnings which every existent compiler produces. Some of those warnings are<br>
>> really dumb. For instance, /W3 also enables C4800, which I don't think we<br>
>> will ever want to implement.<br>
><br>
><br>
> I agree that warning (forcing value to bool) is really, really bad, and I<br>
> hope most serious projects turn it off.<br>
><br>
> However, I can say it's pretty annoying if you're working on cross-platform<br>
> project and have to push to another platform to build just to get some<br>
> compiler warnings for a compiler which you're expected to keep warning<br>
> clean, value judgements about its diagnostic quality aside.  That cycle is<br>
> long and painful, and if you skip it people often yell at you.<br>
><br>
> While clang will never be able to paper over the differences between<br>
> platforms, having some off-by-default diagnostics like this could save users<br>
> some time and increase the likelihood that their changes Just Work on the<br>
> first push.<br>
</div></div></blockquote></div><br></div>