jdenny added a comment.

In D59712#1469301 <https://reviews.llvm.org/D59712#1469301>, @lebedev.ri wrote:

> In D59712#1469295 <https://reviews.llvm.org/D59712#1469295>, @craig.topper 
> wrote:
>
> > Wondering if it would be better to assert for asking for the sign of an 
> > unsigned APSInt. I could see a caller just wanting to get the msb for some 
> > reason and not knowing that isNegative won’t work.
>
>
> Yes, i, too, would think an assert is much better solution (since i literally 
> just tripped over this in this review.)


Imagine a caller wants to check for a negative value for an APSInt from an 
arbitrary expression.  Whether the value is negative is important, but not why. 
 Given that APSInt is documented as knowing its signedness, it seems 
unintuitive that the caller is responsible for first checking for an unsigned 
type to avoid an internal compiler error.

If the fear is that developers are too used to calling isNegative to check the 
high bit, maybe isNegative should always (regardless of signedness) fail an 
assert for APSInt, and we should find different function names that won't cause 
such confusion.

Whatever we do for isNegative, we should probably do something similar for 
isNonNegative as it has the same issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59712/new/

https://reviews.llvm.org/D59712



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to