[llvm-commits] [cfe-commits] [Patch] Update to APSInt

[John McCall]

On Jul 18, 2012, at 2:28 AM, Chandler Carruth wrote:
> On Tue, Jul 17, 2012 at 11:47 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Tue, Jul 17, 2012 at 11:26 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 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*.
> 
> 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...
> 
> APInt has no knowledge of whether its high bit is a sign bit, so always zero-extending will be wrong in the case where it is in fact a sign bit. APSInt does know this, so if we want to support heterogenous comparisons, we should sign-extend if the APSInt is signed, and zero-extend if it is unsigned. Heterogenous comparison on APInt is fundamentally unsafe, so asserting there seems reasonable to me.
> 
> Well, Nick's comment may obviate the extension question, which leaves us with a simpler problem of comparing the same sizes for equality or inequality. I don't actually see any problems comparing same-sized APInts and APSInts for equality or inequality as-if they were both APInts. Given two APSInts, I think that the signedness should participate in the equality test though...

It seems silly for APInt to treat bitwidth inequality as an illegal operation but APSInt to treat it as a semantic difference.  APInt's assertions *do* find bugs;  I would much rather extend those to APSInt than have it forge a new contract.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120718/cd3ec9bd/attachment.html>