CLIUtils/CLI11

unable to customize parse function for double_constructible types in C++20.

nanoric opened this issue · 2 comments

Consider following code:
All works fine with C++14 & C++17.
But failed to call my function in C++20.

Tested under Visual Studio 16.9.0 with "/std:c++latest".

#include <CLI/App.hpp>
#include <CLI/CLI.hpp>
#include <iostream>
#include <sstream>


template <class T = int>
struct Values
{
    T a;
    T b;
    T c;
};

using DoubleValues = Values<double>;


namespace CLI
{
auto &&operator>>(std::istringstream &in, Values<double> &v)
{
    std::string input;
    in >> input;
    std::cout
        << "called correct function " __FUNCTION__ " ! "
                                                   "val: "
        << input << std::endl;
    return in;
}
bool lexical_cast(const std::string &input,
                  Values<double> &v)
{
    std::cout
        << "called correct function " __FUNCTION__ " ! "
                                                   "val: "
        << input << std::endl;
    return true;
}

};// namespace CLI

DoubleValues doubles;
void argparse(CLI::Option_group *group)
{
    group->add_option("--fee-rate", doubles)
        ->envname("FEE_RATE")
        ->default_str("0");
}

int main(int argc, char **argv)
{
    CLI::App app;

    argparse(app.add_option_group("param"));
    CLI11_PARSE(app, argc, argv);
    return 0;
}

The same problem may applies to all object_category that constructible.

Ok, played with this one for a while.
The change between C++20 and C++17 is in the aggregate initialization that was added in C++20. This allows the structure to be declared using an aggregate which does not need to be complete, Hence it can be constructed from a single double in C++20 which was not the case for C++17 or earlier. This means in C++20 it is double_constructible, where it was not in earlier compilers. So it can trigger that overload.

Now why it was not triggering your overload?

because the the type in C++20 now reads as double_constructible it now generates a lexical_cast for it with a perfectly matching signature. Just like your overload. So it goes into argument dependent lookup(ADL) to determine which one to use.

So it tries to find one in the namespace of the arguments. In the code above doesn't find any.
Not going to find any in std either so it looks in the context of the call namely CLI::detail. And finds the double_constructible overload so uses that one.

If we try to put the custom lexical_cast in CLI::detail that doesn't work either because the double_constructible one comes first. So you can move the custom definition into CLI::detail namespace and before the include CLI.hpp then it comes first and works.

The better solution is to move the definition out of the CLI namespace and into the same namespace as DoubleValues then it works as expected.

Bottom line: the type detection in CLI seems to be working correctly, though has changed in C++20. Lexical cast overloads need to be in the same namespace as the custom type for it to work correctly.

#include <CLI/CLI.hpp>
#include <iostream>
#include <sstream>

template <class T = int>
struct Values
{
    T a;
    T b;
    T c;
};

using DoubleValues = Values<double>;

auto &&operator>>(std::istringstream &in, Values<double> &v)
{
    std::string input;
    in >> input;
    std::cout
        << "called correct function " __FUNCTION__ " ! "
                                                   "val: "
        << input << std::endl;
    return in;
}
bool lexical_cast(const std::string &input,
                  Values<double> &v)
{
    std::cout
        << "called correct function " __FUNCTION__ " ! "
                                                   "val: "
        << input << std::endl;
    return true;
}

DoubleValues doubles;
void argparse(CLI::Option_group *group)
{
    group->add_option("--fee-rate", doubles)
        ->envname("FEE_RATE")
        ->default_str("0");
}

int main(int argc, char **argv)
{
    CLI::App app;

    argparse(app.add_option_group("param"));
    CLI11_PARSE(app, argc, argv);
    return 0;
}