Using ADL everywhere for lexical_cast (#820)

* Using ADL everywhere for lexical_cast

* Fixes in docs

* Add a test for old extension mechanism

* style: pre-commit.ci fixes

* Make gcc happy

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Philip Top <phlptp@gmail.com>

README.md

@@ -1435,11 +1435,10 @@ provide a custom `operator>>` with an `istream` (inside the CLI namespace is
 fine if you don't want to interfere with an existing `operator>>`).
 
 If you wanted to extend this to support a completely new type, use a lambda or
-add a specialization of the `lexical_cast` function template in the namespace of
-the type you need to convert to. Some examples of some new parsers for
-`complex<double>` that support all of the features of a standard `add_options`
-call are in [one of the tests](./tests/NewParseTest.cpp). A simpler example is
-shown below:
+add an overload of the `lexical_cast` function in the namespace of the type you
+need to convert to. Some examples of some new parsers for `complex<double>` that
+support all of the features of a standard `add_options` call are in
+[one of the tests](./tests/NewParseTest.cpp). A simpler example is shown below:
 
 #### Example
 

book/chapters/internals.md

@@ -8,9 +8,9 @@ classes or inheritance. This is accomplished through lambda functions.
 This looks like:
 
 ```cpp
-Option* add_option(string name, T item) {
+Option* add_option(string name, T &item) {
     this->function = [&item](string value){
-        item = detail::lexical_cast<T>(value);
+        return lexical_cast(value, item);
     }
 }
 ```

include/CLI/App.hpp

@@ -626,7 +626,8 @@ class App {
                      std::string flag_description = "") {
 
         CLI::callback_t fun = [&flag_result](const CLI::results_t &res) {
-            return CLI::detail::lexical_cast(res[0], flag_result);
+            using CLI::detail::lexical_cast;
+            return lexical_cast(res[0], flag_result);
         };
         auto *opt = _add_flag_internal(flag_name, std::move(fun), std::move(flag_description));
         return detail::default_flag_modifiers<T>(opt);
@@ -642,8 +643,9 @@ class App {
         CLI::callback_t fun = [&flag_results](const CLI::results_t &res) {
             bool retval = true;
             for(const auto &elem : res) {
+                using CLI::detail::lexical_cast;
                 flag_results.emplace_back();
-                retval &= detail::lexical_cast(elem, flag_results.back());
+                retval &= lexical_cast(elem, flag_results.back());
             }
             return retval;
         };

include/CLI/TypeTools.hpp

@@ -966,18 +966,18 @@ bool lexical_cast(const std::string &input, T &output) {
     bool worked = false;
     auto nloc = str1.find_last_of("+-");
     if(nloc != std::string::npos && nloc > 0) {
-        worked = detail::lexical_cast(str1.substr(0, nloc), x);
+        worked = lexical_cast(str1.substr(0, nloc), x);
         str1 = str1.substr(nloc);
         if(str1.back() == 'i' || str1.back() == 'j')
             str1.pop_back();
-        worked = worked && detail::lexical_cast(str1, y);
+        worked = worked && lexical_cast(str1, y);
     } else {
         if(str1.back() == 'i' || str1.back() == 'j') {
             str1.pop_back();
-            worked = detail::lexical_cast(str1, y);
+            worked = lexical_cast(str1, y);
             x = XC{0};
         } else {
-            worked = detail::lexical_cast(str1, x);
+            worked = lexical_cast(str1, x);
             y = XC{0};
         }
     }
@@ -1198,7 +1198,7 @@ template <typename AssignTo,
                       detail::enabler> = detail::dummy>
 bool lexical_assign(const std::string &input, AssignTo &output) {
     ConvertTo val{};
-    bool parse_result = (!input.empty()) ? lexical_cast<ConvertTo>(input, val) : true;
+    bool parse_result = (!input.empty()) ? lexical_cast(input, val) : true;
     if(parse_result) {
         output = val;
     }
@@ -1214,7 +1214,7 @@ template <
                 detail::enabler> = detail::dummy>
 bool lexical_assign(const std::string &input, AssignTo &output) {
     ConvertTo val{};
-    bool parse_result = input.empty() ? true : lexical_cast<ConvertTo>(input, val);
+    bool parse_result = input.empty() ? true : lexical_cast(input, val);
     if(parse_result) {
         output = AssignTo(val);  // use () form of constructor to allow some implicit conversions
     }
@@ -1292,7 +1292,7 @@ bool lexical_conversion(const std::vector<std::string> &strings, AssignTo &outpu
         if(str1.back() == 'i' || str1.back() == 'j') {
             str1.pop_back();
         }
-        auto worked = detail::lexical_cast(strings[0], x) && detail::lexical_cast(str1, y);
+        auto worked = lexical_cast(strings[0], x) && lexical_cast(str1, y);
         if(worked) {
             output = ConvertTo{x, y};
         }
@@ -1556,7 +1556,7 @@ inline std::string sum_string_vector(const std::vector<std::string> &values) {
     std::string output;
     for(const auto &arg : values) {
         double tv{0.0};
-        auto comp = detail::lexical_cast<double>(arg, tv);
+        auto comp = lexical_cast(arg, tv);
         if(!comp) {
             try {
                 tv = static_cast<double>(detail::to_flag_value(arg));

include/CLI/Validators.hpp

@@ -270,8 +270,9 @@ template <typename DesiredType> class TypeValidator : public Validator {
   public:
     explicit TypeValidator(const std::string &validator_name)
         : Validator(validator_name, [](std::string &input_string) {
+              using CLI::detail::lexical_cast;
               auto val = DesiredType();
-              if(!detail::lexical_cast(input_string, val)) {
+              if(!lexical_cast(input_string, val)) {
                   return std::string("Failed parsing ") + input_string + " as a " + detail::type_name<DesiredType>();
               }
               return std::string();
@@ -305,8 +306,9 @@ class Range : public Validator {
         }
 
         func_ = [min_val, max_val](std::string &input) {
+            using CLI::detail::lexical_cast;
             T val;
-            bool converted = detail::lexical_cast(input, val);
+            bool converted = lexical_cast(input, val);
             if((!converted) || (val < min_val || val > max_val)) {
                 std::stringstream out;
                 out << "Value " << input << " not in range [";
@@ -342,8 +344,9 @@ class Bound : public Validator {
         description(out.str());
 
         func_ = [min_val, max_val](std::string &input) {
+            using CLI::detail::lexical_cast;
             T val;
-            bool converted = detail::lexical_cast(input, val);
+            bool converted = lexical_cast(input, val);
             if(!converted) {
                 return std::string("Value ") + input + " could not be converted";
             }
@@ -534,8 +537,9 @@ class IsMember : public Validator {
         // This is the function that validates
         // It stores a copy of the set pointer-like, so shared_ptr will stay alive
         func_ = [set, filter_fn](std::string &input) {
+            using CLI::detail::lexical_cast;
             local_item_t b;
-            if(!detail::lexical_cast(input, b)) {
+            if(!lexical_cast(input, b)) {
                 throw ValidationError(input);  // name is added later
             }
             if(filter_fn) {
@@ -602,8 +606,9 @@ class Transformer : public Validator {
         desc_function_ = [mapping]() { return detail::generate_map(detail::smart_deref(mapping)); };
 
         func_ = [mapping, filter_fn](std::string &input) {
+            using CLI::detail::lexical_cast;
             local_item_t b;
-            if(!detail::lexical_cast(input, b)) {
+            if(!lexical_cast(input, b)) {
                 return std::string();
                 // there is no possible way we can match anything in the mapping if we can't convert so just return
             }
@@ -671,8 +676,9 @@ class CheckedTransformer : public Validator {
         desc_function_ = tfunc;
 
         func_ = [mapping, tfunc, filter_fn](std::string &input) {
+            using CLI::detail::lexical_cast;
             local_item_t b;
-            bool converted = detail::lexical_cast(input, b);
+            bool converted = lexical_cast(input, b);
             if(converted) {
                 if(filter_fn) {
                     b = filter_fn(b);
@@ -774,7 +780,8 @@ class AsNumberWithUnit : public Validator {
                 unit = detail::to_lower(unit);
             }
             if(unit.empty()) {
-                if(!detail::lexical_cast(input, num)) {
+                using CLI::detail::lexical_cast;
+                if(!lexical_cast(input, num)) {
                     throw ValidationError(std::string("Value ") + input + " could not be converted to " +
                                           detail::type_name<Number>());
                 }
@@ -792,7 +799,8 @@ class AsNumberWithUnit : public Validator {
             }
 
             if(!input.empty()) {
-                bool converted = detail::lexical_cast(input, num);
+                using CLI::detail::lexical_cast;
+                bool converted = lexical_cast(input, num);
                 if(!converted) {
                     throw ValidationError(std::string("Value ") + input + " could not be converted to " +
                                           detail::type_name<Number>());

include/CLI/impl/App_inl.hpp

@@ -265,8 +265,9 @@ CLI11_INLINE Option *App::add_flag_callback(std::string flag_name,
                                             std::string flag_description) {
 
     CLI::callback_t fun = [function](const CLI::results_t &res) {
+        using CLI::detail::lexical_cast;
         bool trigger{false};
-        auto result = CLI::detail::lexical_cast(res[0], trigger);
+        auto result = lexical_cast(res[0], trigger);
         if(result && trigger) {
             function();
         }
@@ -281,8 +282,9 @@ App::add_flag_function(std::string flag_name,
                        std::string flag_description) {
 
     CLI::callback_t fun = [function](const CLI::results_t &res) {
+        using CLI::detail::lexical_cast;
         std::int64_t flag_count{0};
-        CLI::detail::lexical_cast(res[0], flag_count);
+        lexical_cast(res[0], flag_count);
         function(flag_count);
         return true;
     };

include/CLI/impl/Config_inl.hpp

@@ -31,8 +31,9 @@ CLI11_INLINE std::string convert_arg_for_ini(const std::string &arg, char string
     }
     // floating point conversion can convert some hex codes, but don't try that here
     if(arg.compare(0, 2, "0x") != 0 && arg.compare(0, 2, "0X") != 0) {
+        using CLI::detail::lexical_cast;
         double val = 0.0;
-        if(detail::lexical_cast(arg, val)) {
+        if(lexical_cast(arg, val)) {
             return arg;
         }
     }

include/CLI/impl/Validators_inl.hpp

@@ -219,7 +219,8 @@ CLI11_INLINE IPV4Validator::IPV4Validator() : Validator("IPV4") {
         }
         int num = 0;
         for(const auto &var : result) {
-            bool retval = detail::lexical_cast(var, num);
+            using CLI::detail::lexical_cast;
+            bool retval = lexical_cast(var, num);
             if(!retval) {
                 return std::string("Failed parsing number (") + var + ')';
             }

tests/NewParseTest.cpp

@@ -163,14 +163,10 @@ class spair {
     std::string first{};
     std::string second{};
 };
-// an example of custom converter that can be used to add new parsing options
-// On MSVC and possibly some other new compilers this can be a free standing function without the template
-// specialization but this is compiler dependent
-namespace CLI {
-namespace detail {
-
-template <> bool lexical_cast<spair>(const std::string &input, spair &output) {
 
+// Example of a custom converter that can be used to add new parsing options.
+// It will be found via argument-dependent lookup, so should be in the same namespace as the `spair` type.
+bool lexical_cast(const std::string &input, spair &output) {
     auto sep = input.find_first_of(':');
     if((sep == std::string::npos) && (sep > 0)) {
         return false;
@@ -178,8 +174,6 @@ template <> bool lexical_cast<spair>(const std::string &input, spair &output) {
     output = {input.substr(0, sep), input.substr(sep + 1)};
     return true;
 }
-}  // namespace detail
-}  // namespace CLI
 
 TEST_CASE_METHOD(TApp, "custom_string_converter", "[newparse]") {
     spair val;
@@ -201,6 +195,96 @@ TEST_CASE_METHOD(TApp, "custom_string_converterFail", "[newparse]") {
     CHECK_THROWS_AS(run(), CLI::ConversionError);
 }
 
+/// Wrapper with an unconvenient interface
+template <class T> class badlywrapped {
+  public:
+    badlywrapped() : value() {}
+
+    CLI11_NODISCARD T get() const { return value; }
+
+    void set(T val) { value = val; }
+
+  private:
+    T value;
+};
+
+// Example of a custom converter for a template type.
+// It will be found via argument-dependent lookup, so should be in the same namespace as the `badlywrapped` type.
+template <class T> bool lexical_cast(const std::string &input, badlywrapped<T> &output) {
+    // This using declaration lets us use an unqualified call to lexical_cast below. This is important because
+    // unqualified call finds the proper overload via argument-dependent lookup, and thus it will be able to find
+    // an overload for `spair` type, which is not in `CLI::detail`.
+    using CLI::detail::lexical_cast;
+
+    T value;
+    if(!lexical_cast(input, value))
+        return false;
+    output.set(value);
+    return true;
+}
+
+TEST_CASE_METHOD(TApp, "custom_string_converter_flag", "[newparse]") {
+    badlywrapped<bool> val;
+    std::vector<badlywrapped<bool>> vals;
+    app.add_flag("-1", val);
+    app.add_flag("-2", vals);
+
+    val.set(false);
+    args = {"-1"};
+    run();
+    CHECK(true == val.get());
+
+    args = {"-2", "-2"};
+    run();
+    CHECK(2 == vals.size());
+    CHECK(true == vals[0].get());
+    CHECK(true == vals[1].get());
+}
+
+TEST_CASE_METHOD(TApp, "custom_string_converter_adl", "[newparse]") {
+    // This test checks that the lexical_cast calls route as expected.
+    badlywrapped<spair> val;
+
+    app.add_option("-d,--dual_string", val);
+
+    args = {"-d", "string1:string2"};
+
+    run();
+    CHECK("string1" == val.get().first);
+    CHECK("string2" == val.get().second);
+}
+
+/// Another wrapper to test that specializing CLI::detail::lexical_cast works
+struct anotherstring {
+    anotherstring() = default;
+    std::string s{};
+};
+
+// This is a custom converter done via specializing the CLI::detail::lexical_cast template. This was the recommended
+// mechanism for extending the library before, so we need to test it. Don't do this in your code, use
+// argument-dependent lookup as outlined in the examples for spair and template badlywrapped.
+namespace CLI {
+namespace detail {
+template <> bool lexical_cast<anotherstring>(const std::string &input, anotherstring &output) {
+    bool result = lexical_cast(input, output.s);
+    if(result)
+        output.s += "!";
+    return result;
+}
+}  // namespace detail
+}  // namespace CLI
+
+TEST_CASE_METHOD(TApp, "custom_string_converter_specialize", "[newparse]") {
+    anotherstring s;
+
+    app.add_option("-s", s);
+
+    args = {"-s", "something"};
+
+    run();
+    CHECK("something!" == s.s);
+}
+
 /// simple class to wrap another  with a very specific type constructor and assignment operators to test out some of the
 /// option assignments
 template <class X> class objWrapper {