diff --git a/src/mathicgb/MonoMonoid.hpp b/src/mathicgb/MonoMonoid.hpp index 8d51dda..a5b537f 100644 --- a/src/mathicgb/MonoMonoid.hpp +++ b/src/mathicgb/MonoMonoid.hpp @@ -83,7 +83,7 @@ namespace MonoMonoidInternal { bool varsReversed() const {return mVarsReversed;} protected: - typedef std::vector HashCoefficients; + typedef std::vector HashCoefficients; typedef std::vector Gradings; static Gradings makeGradings(const Order& order) { @@ -627,7 +627,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { // See computeHash() for an explanation of all the casts. const auto hashA = static_cast(hash(a)); const auto hashB = static_cast(hash(b)); - return static_cast(static_cast(hashA + hashB)); + return static_cast(toExponent(hashA + hashB)); } /// Returns true if all the exponents of mono are zero. In other @@ -958,7 +958,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { degrees[grading] = monoidFrom.degree(from, grading); } if (StoreHash) - access(to, hashIndex()) = monoidFrom.hash(from); + access(to, hashIndex()) = toExponent(monoidFrom.hash(from)); MATHICGB_ASSERT(debugValid(to)); // todo: check equal @@ -1036,8 +1036,8 @@ class MonoMonoid : private MonoMonoidInternal::Base { MATHICGB_ASSERT(debugValid(a)); MATHICGB_ASSERT(debugValid(b)); - for (auto i = lastEntryIndex(); i != beforeEntriesIndexBegin(); --i) - access(prod, i) = access(a, i) + access(b, i); + for (auto i = entriesIndexBegin(); i < entriesIndexEnd(); ++i) + access(prod, i) = addWrap(access(a, i), access(b, i)); MATHICGB_ASSERT(debugValid(prod)); } @@ -1048,7 +1048,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { MATHICGB_ASSERT(debugValid(prod)); for (auto i = entriesIndexBegin(); i < entriesIndexEnd(); ++i) - access(prod, i) += access(a, i); + access(prod, i) = addWrap(access(prod, i), access(a, i)); MATHICGB_ASSERT(debugValid(prod)); } @@ -1060,7 +1060,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { MATHICGB_ASSERT(debugValid(by)); for (auto i = entriesIndexBegin(); i < entriesIndexEnd(); ++i) - access(quo, i) = access(num, i) - access(by, i); + access(quo, i) = subtractWrap(access(num, i), access(by, i)); MATHICGB_ASSERT(debugValid(quo)); } @@ -1072,7 +1072,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { MATHICGB_ASSERT(debugValid(num)); for (auto i = entriesIndexBegin(); i < entriesIndexEnd(); ++i) - access(num, i) -= access(by, i); + access(num, i) = subtractWrap(access(num, i), access(by, i)); MATHICGB_ASSERT(debugValid(num)); } @@ -1089,7 +1089,7 @@ class MonoMonoid : private MonoMonoidInternal::Base { ); for (auto i = entriesIndexBegin(); i < entriesIndexEnd(); ++i) - access(quo, i) = access(num, i) - access(by, i); + access(quo, i) = subtractWrap(access(num, i), access(by, i)); MATHICGB_ASSERT(debugValid(quo)); } @@ -1335,7 +1335,12 @@ class MonoMonoid : private MonoMonoidInternal::Base { } bool isNull() const {return mMono.isNull();} - void toNull() {mPool->free(std::move(*this));} + void toNull() { + // A Mono with no pool owns nothing, so there is nothing to free. + MATHICGB_ASSERT(mPool != nullptr || isNull()); + if (mPool != nullptr) + mPool->free(std::move(*this)); + } MonoPtr ptr() {return mMono;} ConstMonoPtr ptr() const {return mMono;} @@ -1881,24 +1886,50 @@ class MonoMonoid : private MonoMonoidInternal::Base { return true; } + /// HashValue widened to at least unsigned int, as integer promotion would + /// turn a narrower type back into a signed int that can overflow. + typedef typename std::common_type::type + UnsignedExponent; + + static Exponent toExponent(const UnsignedExponent value) { + static_assert( + static_cast(static_cast(-1)) == + static_cast(-1), + "Converting an out of range unsigned value to an Exponent must wrap " + "around." + ); + return static_cast(value); + } + + static UnsignedExponent toUnsigned(const Exponent value) { + return static_cast(value); + } + + /// Returns a+b and a-b modulo 2^n, as signed overflow is undefined behavior. + static Exponent addWrap(const Exponent a, const Exponent b) { + return toExponent(toUnsigned(a) + toUnsigned(b)); + } + + static Exponent subtractWrap(const Exponent a, const Exponent b) { + return toExponent(toUnsigned(a) - toUnsigned(b)); + } + HashValue computeHash(ConstMonoRef mono) const { - HashValue hash = HasComponent ? component(mono) : 0; - for (VarIndex var = 0; var < varCount(); ++var) { - hash += - static_cast(exponent(mono, var)) * hashCoefficients()[var]; - } + UnsignedExponent hash = HasComponent ? component(mono) : 0; + for (VarIndex var = 0; var < varCount(); ++var) + hash += toUnsigned(exponent(mono, var)) * hashCoefficients()[var]; // Hash values are stored as exponents. If the cast to an exponent // changes the value, then we need computeHashValue to match that // change by casting to an exponent and back. Otherwise the computed // hash value will not match a hash value that has been stored. - return static_cast(static_cast(hash)); + return static_cast(toExponent(hash)); } void setHash(MonoRef mono) const { if (!StoreHash) return; - rawPtr(mono)[hashIndex()] = computeHash(mono); + rawPtr(mono)[hashIndex()] = toExponent(computeHash(mono)); MATHICGB_ASSERT(debugHashValid(mono)); } @@ -1909,7 +1940,10 @@ class MonoMonoid : private MonoMonoidInternal::Base { ) const { if (!StoreHash) return; - rawPtr(mono)[hashIndex()] += newComponent - oldComponent; + auto& hash = rawPtr(mono)[hashIndex()]; + hash = toExponent( + toUnsigned(hash) + toUnsigned(newComponent) - toUnsigned(oldComponent) + ); MATHICGB_ASSERT(debugHashValid(mono)); } @@ -1922,8 +1956,12 @@ class MonoMonoid : private MonoMonoidInternal::Base { if (!StoreHash) return; MATHICGB_ASSERT(var < varCount()); - rawPtr(mono)[hashIndex()] += - (newExponent - oldExponent) * hashCoefficients()[var]; + auto& hash = rawPtr(mono)[hashIndex()]; + hash = toExponent( + toUnsigned(hash) + + (toUnsigned(newExponent) - toUnsigned(oldExponent)) * + hashCoefficients()[var] + ); MATHICGB_ASSERT(debugHashValid(mono)); } @@ -1952,7 +1990,6 @@ class MonoMonoid : private MonoMonoidInternal::Base { VarIndex entriesIndexBegin() const {return 0;} VarIndex entriesIndexEnd() const {return entryCount();} VarIndex beforeEntriesIndexBegin() const {return entriesIndexBegin() - 1;} - VarIndex lastEntryIndex() const {return entriesIndexEnd() - 1;} using Base::hashCoefficients; diff --git a/src/mathicgb/RawVector.hpp b/src/mathicgb/RawVector.hpp index b36f1cc..cdd2375 100644 --- a/src/mathicgb/RawVector.hpp +++ b/src/mathicgb/RawVector.hpp @@ -226,6 +226,8 @@ class RawVector { // memcpy onto the end of the vector. void memcpy(const T* from, size_t countOfT) { MATHICGB_ASSERT(countOfT <= capacityToGo()); + if (countOfT == 0) + return; // std::memcpy requires valid pointers even for a zero count std::memcpy(mEnd, from, countOfT * sizeof(T)); mEnd += countOfT; }