Use signed arithmetic in enumeration helper

With a signed enum and the unsigned step, implicit conversion to unsigned
could change the value in the subtraction operator. Note that errors could
also occur with negative results in pure unsigned operations.

A negative result isn't expected for increasing enumerators, but the boost
iterator implementation does an == comparison that invokes the diff operator
and compares it to 0, and this error actually occurs there in unit tests,
and is caught by UBSAN.

Added some SFINAE to disable accidental instantiation of non-enum
types - before C++ 20 std::underlying_type on a non-enum is undefined.

diff --git a/src/gromacs/utility/enumerationhelpers.h b/src/gromacs/utility/enumerationhelpers.h
index e9119e27ddfc5e4caf6e45c688bc88d5b59ea309..33cc0d2a09a5af98f739893a10cfcd49a500564f 100644 (file)
--- a/src/gromacs/utility/enumerationhelpers.h
+++ b/src/gromacs/utility/enumerationhelpers.h
@@ -112,11 +112,12 @@ namespace gmx
  * \tparam  Last       Last constant or number thereof (assumes a default 'Count' member).
  * \tparam  Step       Step increment.
  */
-template<typename EnumType, EnumType Last = EnumType::Count, unsigned int Step = 1>
+template<typename EnumType, EnumType Last = EnumType::Count, std::ptrdiff_t Step = 1>
 class EnumerationIterator final :
     public boost::stl_interfaces::iterator_interface<EnumerationIterator<EnumType, Last, Step>, std::random_access_iterator_tag, EnumType>
 {
 public:
+    static_assert(std::is_enum_v<EnumType>, "Enumeration iterator must be over an enum type.");
     //! Convenience alias
     using IntegerType = std::underlying_type_t<EnumType>;
 
@@ -143,7 +144,7 @@ public:
     //! Difference operator
     constexpr std::ptrdiff_t operator-(const EnumerationIterator other) const noexcept
     {
-        return (m_current - other.m_current) / Step;
+        return (static_cast<std::ptrdiff_t>(m_current) - static_cast<std::ptrdiff_t>(other.m_current)) / Step;
     }
 
 private: