[llvm] r233067 - Silencing some MSVC warnings "C4805: '^' : unsafe mix of type 'bool' and type 'unsigned int' in operation"; NFC.

[David Blaikie]

On Tue, Mar 24, 2015 at 9:18 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Tue, Mar 24, 2015 at 12:13 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >
> >
> > On Tue, Mar 24, 2015 at 5:47 AM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> Author: aaronballman
> >> Date: Tue Mar 24 07:47:51 2015
> >> New Revision: 233067
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=233067&view=rev
> >> Log:
> >> Silencing some MSVC warnings "C4805: '^' : unsafe mix of type 'bool' and
> >> type 'unsigned int' in operation"; NFC.
> >
> >
> > FWIW, Andy Kaylor's started a review/discussion on disabling some MSVC
> > warnings and cleaning up others to get a baseline.
> >
> > This is one I'm sort of inclined to disable (as mentioned in the review
> > thread & some other recent discussions - the basic bar that's certainly
> been
> > applied to GCC and I think is a reasonable one is: If the warning doesn't
> > meet the quality bar to be added to Clang, then it probably doesn't meet
> the
> > quality bar to be enabled for building LLVM projects and we should just
> > disable the warning rather than fixing it even once)
>
> I've not looked at what code is generated by MSVC for this construct,
> but when the warning says "unsafe" in it, I get a bit worried. I'd be
> opposed to silencing this warning if it turns out this really is an
> unsafe operation with MSVC (though I cannot fathom why it would be
> since bool can be implicitly converted to int with well-defined
> semantics). If the operation is safe in MSVC and the warning is more
> stylistic in nature, then I agree this would be one we can silence
> globally.
>

Looking at the docs (sparse as they are) on MSDN
https://msdn.microsoft.com/en-us/library/1hy2y0bk%28v=vs.90%29.aspx perhaps
this warning was intended for the comparison case & spilled over into
assignment?

In the comparison case it's still not going to explode on MSVC in
new/interesting ways, by the sounds of it. It's just seems to be talking
about "int == bool" comparison promoting the bool to an int, then comparing
the ints, rather than the other way around, which perhaps the user intended.

None of the first page of Google results for C4805 seem to indicate
anything more sinister going on here.


>
> In the meantime, I wanted to get the build back to warning-free.
>
> ~Aaron
>
> >
> >>
> >>
> >> Modified:
> >>     llvm/trunk/lib/Support/APFloat.cpp
> >>
> >> Modified: llvm/trunk/lib/Support/APFloat.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=233067&r1=233066&r2=233067&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Support/APFloat.cpp (original)
> >> +++ llvm/trunk/lib/Support/APFloat.cpp Tue Mar 24 07:47:51 2015
> >> @@ -1430,7 +1430,7 @@ APFloat::addOrSubtractSignificand(const
> >>
> >>    /* Determine if the operation on the absolute values is effectively
> >>       an addition or subtraction.  */
> >> -  subtract ^= sign ^ rhs.sign;
> >> +  subtract ^= static_cast<bool>(sign ^ rhs.sign);
> >>
> >>    /* Are we bigger exponent-wise than the RHS?  */
> >>    bits = exponent - rhs.exponent;
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150324/8abf46d9/attachment.html>