fix: wrap min and max for MSVC (#741)

* wrap min and max in parenthesis for Visual studio 2017 and earlier compatibility

* try pre-commit to detect min/max issues

* actually fix the identified issues from the new check

* more min/max fixes

.pre-commit-config.yaml

@@ -71,6 +71,14 @@ repos:
         entry: PyBind|Numpy|Cmake|CCache|PyTest|Github
         exclude: .pre-commit-config.yaml
 
+  - repo: local
+    hooks:
+      - id: avoid-msvc-macro
+        name: Avoid MSVC <=2017 min/max macro (use extra parens)
+        language: pygrep
+        entry: \b(min|max)\(
+        exclude: .pre-commit-config.yaml
+
   - repo: https://github.com/codespell-project/codespell
     rev: v2.1.0
     hooks:

include/CLI/TypeTools.hpp

@@ -1542,8 +1542,8 @@ inline std::string sum_string_vector(const std::vector<std::string> &values) {
             output.append(arg);
         }
     } else {
-        if(val <= static_cast<double>(std::numeric_limits<std::int64_t>::min()) ||
+        if(val <= static_cast<double>((std::numeric_limits<std::int64_t>::min)()) ||
-           val >= static_cast<double>(std::numeric_limits<std::int64_t>::max()) ||
+           val >= static_cast<double>((std::numeric_limits<std::int64_t>::max)()) ||
            val == static_cast<std::int64_t>(val)) {
             output = detail::value_string(static_cast<int64_t>(val));
         } else {

include/CLI/Validators.hpp

@@ -527,7 +527,7 @@ class Range : public Validator {
 /// Check for a non negative number
 const Range NonNegativeNumber((std::numeric_limits<double>::max)(), "NONNEGATIVE");
 
-/// Check for a positive valued number (val>0.0), min() her is the smallest positive number
+/// Check for a positive valued number (val>0.0), <double>::min  here is the smallest positive number
 const Range PositiveNumber((std::numeric_limits<double>::min)(), (std::numeric_limits<double>::max)(), "POSITIVE");
 
 /// Produce a bounded range (factory). Min and max are inclusive.

tests/HelpersTest.cpp

@@ -541,68 +541,68 @@ TEST_CASE("CheckedMultiply: Int", "[helpers]") {
     REQUIRE(CLI::detail::checked_multiply(a, b));
     REQUIRE(0 == a);
 
-    a = std::numeric_limits<int>::max();
+    a = (std::numeric_limits<int>::max)();
     b = 1;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::max() == a);
+    REQUIRE((std::numeric_limits<int>::max)() == a);
 
-    a = std::numeric_limits<int>::max();
+    a = (std::numeric_limits<int>::max)();
     b = 2;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::max() == a);
+    REQUIRE((std::numeric_limits<int>::max)() == a);
 
-    a = std::numeric_limits<int>::max();
+    a = (std::numeric_limits<int>::max)();
     b = -1;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(-std::numeric_limits<int>::max() == a);
+    REQUIRE(-(std::numeric_limits<int>::max)() == a);
 
-    a = std::numeric_limits<int>::max();
+    a = (std::numeric_limits<int>::max)();
-    b = std::numeric_limits<int>::max();
+    b = (std::numeric_limits<int>::max)();
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::max() == a);
+    REQUIRE((std::numeric_limits<int>::max)() == a);
 
-    a = std::numeric_limits<int>::min();
+    a = (std::numeric_limits<int>::min)();
-    b = std::numeric_limits<int>::max();
+    b = (std::numeric_limits<int>::max)();
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::min() == a);
+    REQUIRE((std::numeric_limits<int>::min)() == a);
 
-    a = std::numeric_limits<int>::min();
+    a = (std::numeric_limits<int>::min)();
     b = 1;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::min() == a);
+    REQUIRE((std::numeric_limits<int>::min)() == a);
 
-    a = std::numeric_limits<int>::min();
+    a = (std::numeric_limits<int>::min)();
     b = -1;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::min() == a);
+    REQUIRE((std::numeric_limits<int>::min)() == a);
 
-    b = std::numeric_limits<int>::min();
+    b = (std::numeric_limits<int>::min)();
     a = -1;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
     REQUIRE(-1 == a);
 
-    a = std::numeric_limits<int>::min() / 100;
+    a = (std::numeric_limits<int>::min)() / 100;
     b = 99;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::min() / 100 * 99 == a);
+    REQUIRE((std::numeric_limits<int>::min)() / 100 * 99 == a);
 
-    a = std::numeric_limits<int>::min() / 100;
+    a = (std::numeric_limits<int>::min)() / 100;
     b = -101;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<int>::min() / 100 == a);
+    REQUIRE((std::numeric_limits<int>::min)() / 100 == a);
     a = 2;
-    b = std::numeric_limits<int>::min() / 2;
+    b = (std::numeric_limits<int>::min)() / 2;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    a = std::numeric_limits<int>::min() / 2;
+    a = (std::numeric_limits<int>::min)() / 2;
     b = 2;
     REQUIRE(CLI::detail::checked_multiply(a, b));
 
     a = 4;
-    b = std::numeric_limits<int>::min() / 4;
+    b = (std::numeric_limits<int>::min)() / 4;
     REQUIRE(CLI::detail::checked_multiply(a, b));
 
     a = 48;
-    b = std::numeric_limits<int>::min() / 48;
+    b = (std::numeric_limits<int>::min)() / 48;
     REQUIRE(CLI::detail::checked_multiply(a, b));
 }
 
@@ -622,25 +622,25 @@ TEST_CASE("CheckedMultiply: SizeT", "[helpers]") {
     REQUIRE(CLI::detail::checked_multiply(a, b));
     REQUIRE(0u == a);
 
-    a = std::numeric_limits<std::size_t>::max();
+    a = (std::numeric_limits<std::size_t>::max)();
     b = 1u;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<std::size_t>::max() == a);
+    REQUIRE((std::numeric_limits<std::size_t>::max)() == a);
 
-    a = std::numeric_limits<std::size_t>::max();
+    a = (std::numeric_limits<std::size_t>::max)();
     b = 2u;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<std::size_t>::max() == a);
+    REQUIRE((std::numeric_limits<std::size_t>::max)() == a);
 
-    a = std::numeric_limits<std::size_t>::max();
+    a = (std::numeric_limits<std::size_t>::max)();
-    b = std::numeric_limits<std::size_t>::max();
+    b = (std::numeric_limits<std::size_t>::max)();
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<std::size_t>::max() == a);
+    REQUIRE((std::numeric_limits<std::size_t>::max)() == a);
 
-    a = std::numeric_limits<std::size_t>::max() / 100;
+    a = (std::numeric_limits<std::size_t>::max)() / 100;
     b = 99u;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<std::size_t>::max() / 100u * 99u == a);
+    REQUIRE((std::numeric_limits<std::size_t>::max)() / 100u * 99u == a);
 }
 
 TEST_CASE("CheckedMultiply: Float", "[helpers]") {
@@ -664,30 +664,30 @@ TEST_CASE("CheckedMultiply: Float", "[helpers]") {
     REQUIRE(CLI::detail::checked_multiply(a, b));
     REQUIRE(-INFINITY == Approx(a));
 
-    a = std::numeric_limits<float>::max() / 100.0F;
+    a = (std::numeric_limits<float>::max)() / 100.0F;
     b = 1.0F;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<float>::max() / 100.0F == Approx(a));
+    REQUIRE((std::numeric_limits<float>::max)() / 100.0F == Approx(a));
 
-    a = std::numeric_limits<float>::max() / 100.0F;
+    a = (std::numeric_limits<float>::max)() / 100.0F;
     b = 99.0F;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<float>::max() / 100.0F * 99.0F == Approx(a));
+    REQUIRE((std::numeric_limits<float>::max)() / 100.0F * 99.0F == Approx(a));
 
-    a = std::numeric_limits<float>::max() / 100.0F;
+    a = (std::numeric_limits<float>::max)() / 100.0F;
     b = 101;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<float>::max() / 100.0F == Approx(a));
+    REQUIRE((std::numeric_limits<float>::max)() / 100.0F == Approx(a));
 
-    a = std::numeric_limits<float>::max() / 100.0F;
+    a = (std::numeric_limits<float>::max)() / 100.0F;
     b = -99;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<float>::max() / 100.0F * -99.0F == Approx(a));
+    REQUIRE((std::numeric_limits<float>::max)() / 100.0F * -99.0F == Approx(a));
 
-    a = std::numeric_limits<float>::max() / 100.0F;
+    a = (std::numeric_limits<float>::max)() / 100.0F;
     b = -101;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<float>::max() / 100.0F == Approx(a));
+    REQUIRE((std::numeric_limits<float>::max)() / 100.0F == Approx(a));
 }
 
 TEST_CASE("CheckedMultiply: Double", "[helpers]") {
@@ -711,30 +711,30 @@ TEST_CASE("CheckedMultiply: Double", "[helpers]") {
     REQUIRE(CLI::detail::checked_multiply(a, b));
     REQUIRE(-INFINITY == Approx(a));
 
-    a = std::numeric_limits<double>::max() / 100;
+    a = (std::numeric_limits<double>::max)() / 100;
     b = 1;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<double>::max() / 100 == Approx(a));
+    REQUIRE((std::numeric_limits<double>::max)() / 100 == Approx(a));
 
-    a = std::numeric_limits<double>::max() / 100;
+    a = (std::numeric_limits<double>::max)() / 100;
     b = 99;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<double>::max() / 100 * 99 == Approx(a));
+    REQUIRE((std::numeric_limits<double>::max)() / 100 * 99 == Approx(a));
 
-    a = std::numeric_limits<double>::max() / 100;
+    a = (std::numeric_limits<double>::max)() / 100;
     b = 101;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<double>::max() / 100 == Approx(a));
+    REQUIRE((std::numeric_limits<double>::max)() / 100 == Approx(a));
 
-    a = std::numeric_limits<double>::max() / 100;
+    a = (std::numeric_limits<double>::max)() / 100;
     b = -99;
     REQUIRE(CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<double>::max() / 100 * -99 == Approx(a));
+    REQUIRE((std::numeric_limits<double>::max)() / 100 * -99 == Approx(a));
 
-    a = std::numeric_limits<double>::max() / 100;
+    a = (std::numeric_limits<double>::max)() / 100;
     b = -101;
     REQUIRE(!CLI::detail::checked_multiply(a, b));
-    REQUIRE(std::numeric_limits<double>::max() / 100 == Approx(a));
+    REQUIRE((std::numeric_limits<double>::max)() / 100 == Approx(a));
 }
 
 // Yes, this is testing an app_helper :)
@@ -1029,11 +1029,11 @@ TEST_CASE("Types: TypeName", "[helpers]") {
 
 TEST_CASE("Types: OverflowSmall", "[helpers]") {
     signed char x;
-    auto strmax = std::to_string(std::numeric_limits<signed char>::max() + 1);
+    auto strmax = std::to_string((std::numeric_limits<signed char>::max)() + 1);
     CHECK_FALSE(CLI::detail::lexical_cast(strmax, x));
 
     unsigned char y;
-    strmax = std::to_string(std::numeric_limits<unsigned char>::max() + 1);
+    strmax = std::to_string((std::numeric_limits<unsigned char>::max)() + 1);
     CHECK_FALSE(CLI::detail::lexical_cast(strmax, y));
 }
 
@@ -1051,7 +1051,7 @@ TEST_CASE("Types: LexicalCastInt", "[helpers]") {
     CHECK_FALSE(CLI::detail::lexical_cast(signed_input, x_unsigned));
 
     unsigned char y;
-    std::string overflow_input = std::to_string(std::numeric_limits<uint64_t>::max()) + "0";
+    std::string overflow_input = std::to_string((std::numeric_limits<uint64_t>::max)()) + "0";
     CHECK_FALSE(CLI::detail::lexical_cast(overflow_input, y));
 
     char y_signed;
@@ -1078,7 +1078,7 @@ TEST_CASE("Types: LexicalCastDouble", "[helpers]") {
     std::string bad_input = "hello";
     CHECK_FALSE(CLI::detail::lexical_cast(bad_input, x));
 
-    std::string overflow_input = "1" + std::to_string(std::numeric_limits<long double>::max());
+    std::string overflow_input = "1" + std::to_string((std::numeric_limits<long double>::max)());
     CHECK(CLI::detail::lexical_cast(overflow_input, x));
     CHECK_FALSE(std::isfinite(x));