On Sun, Jul 22, 2012 at 7:39 AM, Chris Lattner <[email protected]> wrote:
> > On Jul 18, 2012, at 6:16 PM, Richard Trieu wrote: > > On Wed, Jul 18, 2012 at 11:19 AM, Chandler Carruth > <[email protected]>wrote: > >> On Wed, Jul 18, 2012 at 11:12 AM, John McCall <[email protected]> wrote: >> >>> On Jul 18, 2012, at 2:28 AM, Chandler Carruth wrote: >>> >>> On Tue, Jul 17, 2012 at 11:47 PM, Richard Smith >>> <[email protected]>wrote: >>> >>>> On Tue, Jul 17, 2012 at 11:26 PM, Chandler Carruth < >>>> [email protected]> 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. >>> >> >> I never intended to suggest inconsistency between the two. I just didn't >> understand the motivation for the assertion even at the APInt layer. Nick >> provided that though, which was all I needed. =] >> >> New patch. Switched int parameters to int64_t. APSInt::operator!= now > refers to APSInt::operator== > > > LGTM, thanks! > > -Chris > > Committed at r160641 and r160642.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
