[LLVMdev] Enabling stricter warnings for Windows builds

[David Blaikie]

On Mon, Mar 23, 2015 at 5:03 PM, Kaylor, Andrew <andrew.kaylor at intel.com>
wrote:

>  >> warning C4706: assignment within conditional expression (296 times)
>
> > Only 300? I was going to say that I assume this warning doesn't support
> the double-paren suppression that GCC/Clang's -Wparentheses dictate, but
> with such a low hit rate I wonder if it's some other sub-set of cases MSVC
> is seeing here. (or maybe we don't use even parentheses-bound assignment in
> conditionals very often at all)
>
>
>
> Yeah, I expected more too.  I haven’t looked at any of these to see what
> might explain it.
>
>
>
>
>
> >> warning C4389: signed/unsigned mismatch (28 times)
>
> > Wonder how this is different from C4245?
>
>
>
> C4245 seems to apply specifically to const values that the compiler knows
> to have a negative value.
>
>
>
> See examples: https://msdn.microsoft.com/en-us/library/e9s7thk1.aspx
>
>
>
>
>
> >> warning C4189: local variable is initialized but not referenced (6
> times)
>
> > Clang diagnoses cases like this already - I wonder what subset MSVC is
> catching that Clang is not. If it's types with non-trivial
> construction/destruction, then this seems like a warning we might not want
> to enable - it might false-positive on scoped devices, etc.
>
>
>
> Some of these were cases where the code was getting an object pointer and
> then using it to call a static function.  The other cause was places where
> a template parameter in a compound condition had the potential to short
> circuit evaluation of the condition and thus skip the only use of the local
> variable.
>
>
>
> In the former case, fixing the code to avoid the warning seems like an
> improvement.
>

Yeah, sounds fine - but also doesn't sound like it's worth holding as a bar
for LLVM, so I'd still be sort of inclined to disable the warning rather
than produce noise for MSVC-using developers that the rest of the community
isn't really going to worry about (we'd never implement such a warning in
Clang, for example - I doubt it'd even be worth a clang-tidy warning, but
it'd be on the edge)


>   I don’t know if MSVC only warns in the second case if the template is
> actually instantiated with a value that short circuits the condition.  If
> not, that one definitely seems like a nuisance but it was easy enough to
> fix in the places where it was being reported.
>

*nod*


>
>
>
>
> >> warning C4611: interaction between '_setjmp' and C++ object destruction
> is non-portable (2 times)
>
> > Curious - again, worth comparing whatever false positives to whatever
> holes clang might have, etc.
>
>
>
> This one looks potentially serious to me, but it isn’t obvious what to do
> about it so I ended up adding this to the list of warnings I’m going to
> just disable.  It’s a design issue.
>
>
>
> Both instances of the warning occurred in the same place,
> CrashRecoveryContext::runSafely().
>
>
>
> It may be that the best thing to do here is to just document the
> potentially platform-dependent behavior if it really isn’t consistent
> across the platforms we support.
>

I wonder if the warning is only about the non-trivial function_ref object,
or whether it'd fire there regardless? It'd be interesting to know the
limitations/point of the warning in more detail. But yeah, given the small
number of cases it comes up in & it's not likely to get any worse - we're
hardly going to litter the codebase with setjmp/longjmp - so if the current
use case is safe (I've no idea), probably just disable the warning & move
on.

- David


>
>
> -Andy
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150323/64785702/attachment.html>