<div dir="ltr">On Tue, Aug 13, 2013 at 10:36 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Gao, Yunzhong wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">
Hi Nick,<br>
Thanks for reviewing the patch.<br>
I will change the type conversion to "unsigned char" instead; that should make<br>
the result deterministic.<br>
I feel that this is probably still not an ideal solution because the host machine<br>
might have a different size for "char" type than the target machine, but in the<br>
llvm backend I did not find a way to extract the size of target "char" type. I<br>
would guess that machines with non-8-bit bytes are rare nowadays? Since<br>
the patch is only intended for a corner-case comparing a value with zero, it<br></div>
might suffice using a cast to "unsigned char" or even "(value&  255)".<br>
What do you think?<br>
</blockquote>
<br>
Actually, I was thinking about this some more and isn't the testcase you posted undefined behaviour?<br>
<br>
The C standard specifies that it gets converted from int to char. That conversion will produce an implementation-defined result when the int is not in range of a char. Now for the crazy part -- isn't it true that there is no rule requiring a value in the abstract machine of C (or C++) to actually fit in the number of bits allotted? Given the right setup, you can have char x == 1000 evaluate to true (even assuming 8-bit char), and no undefined behaviour, merely implementation-defined results.<br>

<br>
Maybe I'm crazy?<span class=""><font color="#888888"><br></font></span></blockquote><div><br></div><div>It's implementation-defined, not undefined... and every platform LLVM supports (or is ever likely to support) defines the behavior to be exactly what this patch implements.<br>
<br></div><div>And in fact, there isn't any way to have char x == 1000 to evaluate to true if char is 8 bits: the combination of 6.2.6.2p1 and 6.2.5p9 prohibits it.<br></div><div><br>-Eli<br></div></div></div></div>