Why assert if the signs don't match? I would expect the operator== to simply return false if the signs don't match...<div><br></div><div>Why only accept 'int'? The APInt underlying system naturally would support int64_t.</div>
<div><br></div><div>Why use isSameValue()? To be consistent with APInt's operators, you shouldn't support zero-extending.</div><div><br></div><div>I would test for sign, handle than, and then explicitly delegate to APInt::operator==</div>
<div><br></div><div>Then for operator!=, I would implement it directly in terms of negating operator==.</div><div><br></div><div><br></div><div>I find the definition of APInt's operator== deeply troubling. Why *assert* if the bit widths aren't equal? That doesn't make a lot of sense to me. The function that it calls to actually implement it turns around and considers mismatched bitwdiths to cause *inequality*.</div>
<div><br></div><div>However it seems that there is a very simple definition of equality we could use instead: zero-extended equality for APInt, and sign-extended equality for APSInt. I wonder if there would be general support for making APInt::operator== and APSInt::operator== work in this more rational model...</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 17, 2012 at 4:04 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch adds missing operator== and operator!= to APSInt.  Currently, comparing two APSInt's will compare them with APInt's operator==, which does not check signedness.  This may lead to a problem where a large unsigned APSInt will equal a negative APSInt, such as -1 and 0xffff.<div>

<br></div><div>One patch is to LLVM to add the functions.  The other is to fix up the uses in Clang.</div>
<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></blockquote></div><br></div>