Unintuitive APSInt assignment

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.

-Roman

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.

OK, understood. Just a tip of the iceberg it was.

чт, 2 авг. 2018 г. в 16:17, Richard Smith <richard@metafoo.co.uk>: