[cfe-dev] Unintuitive APSInt assignment

[Richard Smith via cfe-dev]

On Thu, 2 Aug 2018 at 14:41, Roman Popov via cfe-dev <cfe-dev at lists.llvm.org>
wrote:

> Hello,
> I'm working on C++ interpreter that uses APSInt type to represent
> integers. I've just spent an hour debugging a bug the cause of which was
> unintuitive APSInt behavior / method signature.
>
> APSInt assignments are declared as follows:
>
> class LLVM_NODISCARD APSInt : public APInt {
> ...
>   APSInt &operator=(APInt RHS) {
> *    // Retain our current sign.*
>     APInt::operator=(std::move(RHS));
>     return *this;
>   }
>
>   APSInt &operator=(uint64_t RHS) {
> *    // Retain our current sign.*
>     APInt::operator=(RHS);
>     return *this;
>   }
> ...
> };
>
> By looking into header one may think that assignment *always retains the
> sign. *But this is not true, because automatically generated copy
> assignment will copy sign from RHS.
>

Yes, APSInt's interface leaves a lot to be desired. Another trap:

APSInt N(123); // 123-bit unsigned APSInt, initialized to zero

Note that the APSInt constructor defaults to unsigned. But...

APSInt::get(123); // 64-bit *signed* APSInt, initialized to 123

... APSInt::get defaults to signed.


Also:

APInt N(64, 123); // 64-bit APInt, initialized to 123
APSInt N(64, 123); // 64-bit unsigned APSInt, initialized to 0

123 here is interpreted as an "isUnsigned" flag by the APSInt constructor.


Also:

APSInt X = ...;
bool k = X.isNegative(); // oops, returns true if the high bit is set, even
if X is unsigned, because this calls APInt::isNegative.


Also:

APInt A = ...;
APSInt B = ...;
A *= A; // ok
B *= B; // ok
A *= 4; // ok
B *= 4; // error
A *= B; // ok
B *= A; // error


If you have the patience to clean up all the uses, I'm sure improvements to
the interface would be welcomed.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180802/776c49f3/attachment.html>