Fix underflow in out of range check (#88)

* Special case bool in cmp_less to avoid msvc's warnings

* Fix underflow in out of range check

The previous fix addressed bool but it seems that was not the heart
of the issue.

The problem is that (lhs - 1) can underflow.

Another example of the check trigger when it shouldn't is:
uint8_t with min_value set to 0 and a value of uint8_t::max
lhs-1 would underflow and find max (see added test).

The issue happens when lhs == rhs. To avoid that situation: flip the if-else!
`cmp_less(rhs, lhs)`means that rhs is strictly less than lhs, so
the check will not trigger when lhs == rhs.

Also, cleanup the bool special-case, as it is no longer necessary.

Note: for the bug to manifest, it is important that the min
of the enums is customized to be 0, so corresponding enum_range
specializations need to be present in the regression tests.

include/magic_enum.hpp

@@ -280,6 +280,10 @@ constexpr bool cmp_less(L lhs, R rhs) noexcept {
   if constexpr (std::is_signed_v<L> == std::is_signed_v<R>) {
     // If same signedness (both signed or both unsigned).
     return lhs < rhs;
+  } else if constexpr (std::is_same_v<L, bool>) { // bool special case due to msvc's C4804, C4018
+      return static_cast<R>(lhs) < rhs;
+  } else if constexpr (std::is_same_v<R, bool>) { // bool special case due to msvc's C4804, C4018
+      return lhs < static_cast<L>(rhs);
   } else if constexpr (std::is_signed_v<R>) {
     // If 'right' is negative, then result is 'false', otherwise cast & compare.
     return rhs > 0 && lhs < static_cast<std::make_unsigned_t<R>>(rhs);
@@ -383,13 +387,11 @@ constexpr int reflected_min() noexcept {
     static_assert(lhs > (std::numeric_limits<std::int16_t>::min)(), "magic_enum::enum_range requires min must be greater than INT16_MIN.");
     constexpr auto rhs = (std::numeric_limits<U>::min)();
 
-    if constexpr (std::is_same_v<bool, U>) {
+    if constexpr (cmp_less(rhs, lhs)) {
-      return static_cast<int>(rhs);
-    } else if constexpr (cmp_less(lhs, rhs)) {
-      return rhs;
-    } else {
       static_assert(!is_valid<E, value<E, lhs - 1, IsFlags>(0)>(), "magic_enum::enum_range detects enum value smaller than min range size.");
       return lhs;
+    } else {
+      return rhs;
     }
   }
 }
@@ -405,9 +407,7 @@ constexpr int reflected_max() noexcept {
     static_assert(lhs < (std::numeric_limits<std::int16_t>::max)(), "magic_enum::enum_range requires max must be less than INT16_MAX.");
     constexpr auto rhs = (std::numeric_limits<U>::max)();
 
-    if constexpr (std::is_same_v<bool, U>) {
+    if constexpr (cmp_less(lhs, rhs)) {
-      return static_cast<int>(rhs);
-    } else if constexpr (cmp_less(lhs, rhs)) {
       static_assert(!is_valid<E, value<E, lhs + 1, IsFlags>(0)>(), "magic_enum::enum_range detects enum value larger than max range size.");
       return lhs;
     } else {

test/test.cpp

@@ -54,12 +54,30 @@ enum number : unsigned long {
 #endif
 };
 
+enum class MaxUsedAsInvalid : std::uint8_t {
+    ONE,
+    TWO,
+    INVALID = std::numeric_limits<std::uint8_t>::max()
+};
+
 enum class Binary : bool {
   ONE,
   TWO
 };
 
 namespace magic_enum::customize {
+template <>
+struct enum_range<MaxUsedAsInvalid> {
+    static constexpr int min = 0;
+    static constexpr int max = 64;
+};
+
+template <>
+struct enum_range<Binary> {
+    static constexpr int min = 0;
+    static constexpr int max = 64;
+};
+
 template <>
 struct enum_range<number> {
   static constexpr int min = 100;
@@ -341,6 +359,9 @@ TEST_CASE("enum_values") {
 
   constexpr auto& s5 = enum_values<Binary>();
   REQUIRE(s5 == std::array<Binary, 2>{{Binary::ONE, Binary::TWO}});
+
+  constexpr auto& s6 = enum_values<MaxUsedAsInvalid>();
+  REQUIRE(s6 == std::array<MaxUsedAsInvalid, 2>{{MaxUsedAsInvalid::ONE, MaxUsedAsInvalid::TWO}});
 }
 
 TEST_CASE("enum_count") {
@@ -358,6 +379,9 @@ TEST_CASE("enum_count") {
 
   constexpr auto s5 = enum_count<Binary>();
   REQUIRE(s5 == 2);
+
+  constexpr auto s6 = enum_count<MaxUsedAsInvalid>();
+  REQUIRE(s6 == 2);
 }
 
 TEST_CASE("enum_name") {
@@ -427,6 +451,7 @@ TEST_CASE("enum_name") {
     REQUIRE(enum_name<number::four>() == "four");
 
     REQUIRE(enum_name<Binary::ONE>() == "ONE");
+    REQUIRE(enum_name<MaxUsedAsInvalid::ONE>() == "ONE");
   }
 }
 
@@ -664,6 +689,9 @@ TEST_CASE("extrema") {
 
     REQUIRE(magic_enum::detail::reflected_min_v<Binary> == 0);
     REQUIRE(magic_enum::detail::min_v<Binary> == false);
+
+    REQUIRE(magic_enum::detail::reflected_min_v<MaxUsedAsInvalid> == 0);
+    REQUIRE(magic_enum::detail::min_v<MaxUsedAsInvalid> == 0);
   }
 
   SECTION("max") {
@@ -689,6 +717,9 @@ TEST_CASE("extrema") {
 
     REQUIRE(magic_enum::detail::reflected_max_v<Binary> == 1);
     REQUIRE(magic_enum::detail::max_v<Binary> == true);
+
+    REQUIRE(magic_enum::detail::reflected_max_v<MaxUsedAsInvalid> == 64);
+    REQUIRE(magic_enum::detail::max_v<MaxUsedAsInvalid> == 1);
   }
 }
 
@@ -755,4 +786,16 @@ TEST_CASE("cmp_less") {
     REQUIRE_FALSE(cmp_less(uint32_t_min, int64_t_min + offset_int32_t));
     REQUIRE_FALSE(cmp_less(uint64_t_min, int32_t_min + offset_int64_t));
   }
+
+  SECTION("bool, right") {
+    REQUIRE(cmp_less(true, 5));
+    REQUIRE(cmp_less(false, 1));
+    REQUIRE_FALSE(cmp_less(false, -1));
+  }
+
+  SECTION("left, bool") {
+    REQUIRE_FALSE(cmp_less(5, true));
+    REQUIRE_FALSE(cmp_less(1, false));
+    REQUIRE(cmp_less(-1, false));
+  }
 }