<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 24, 2015 at 9:18 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Tue, Mar 24, 2015 at 12:13 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
> On Tue, Mar 24, 2015 at 5:47 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> Author: aaronballman<br>
>> Date: Tue Mar 24 07:47:51 2015<br>
>> New Revision: 233067<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233067&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233067&view=rev</a><br>
>> Log:<br>
>> Silencing some MSVC warnings "C4805: '^' : unsafe mix of type 'bool' and<br>
>> type 'unsigned int' in operation"; NFC.<br>
><br>
><br>
> FWIW, Andy Kaylor's started a review/discussion on disabling some MSVC<br>
> warnings and cleaning up others to get a baseline.<br>
><br>
> This is one I'm sort of inclined to disable (as mentioned in the review<br>
> thread & some other recent discussions - the basic bar that's certainly been<br>
> applied to GCC and I think is a reasonable one is: If the warning doesn't<br>
> meet the quality bar to be added to Clang, then it probably doesn't meet the<br>
> quality bar to be enabled for building LLVM projects and we should just<br>
> disable the warning rather than fixing it even once)<br>
<br>
</span>I've not looked at what code is generated by MSVC for this construct,<br>
but when the warning says "unsafe" in it, I get a bit worried. I'd be<br>
opposed to silencing this warning if it turns out this really is an<br>
unsafe operation with MSVC (though I cannot fathom why it would be<br>
since bool can be implicitly converted to int with well-defined<br>
semantics). If the operation is safe in MSVC and the warning is more<br>
stylistic in nature, then I agree this would be one we can silence<br>
globally.<br></blockquote><div><br>Looking at the docs (sparse as they are) on MSDN <a href="https://msdn.microsoft.com/en-us/library/1hy2y0bk%28v=vs.90%29.aspx">https://msdn.microsoft.com/en-us/library/1hy2y0bk%28v=vs.90%29.aspx</a> perhaps this warning was intended for the comparison case & spilled over into assignment?<br><br>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.<br><br>None of the first page of Google results for C4805 seem to indicate anything more sinister going on here.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
In the meantime, I wanted to get the build back to warning-free.<br>
<span class=""><font color="#888888"><br>
~Aaron<br>
</font></span><div class=""><div class="h5"><br>
><br>
>><br>
>><br>
>> Modified:<br>
>>     llvm/trunk/lib/Support/APFloat.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/Support/APFloat.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=233067&r1=233066&r2=233067&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=233067&r1=233066&r2=233067&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Support/APFloat.cpp (original)<br>
>> +++ llvm/trunk/lib/Support/APFloat.cpp Tue Mar 24 07:47:51 2015<br>
>> @@ -1430,7 +1430,7 @@ APFloat::addOrSubtractSignificand(const<br>
>><br>
>>    /* Determine if the operation on the absolute values is effectively<br>
>>       an addition or subtraction.  */<br>
>> -  subtract ^= sign ^ rhs.sign;<br>
>> +  subtract ^= static_cast<bool>(sign ^ rhs.sign);<br>
>><br>
>>    /* Are we bigger exponent-wise than the RHS?  */<br>
>>    bits = exponent - rhs.exponent;<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>