1

I'm stumped that after some code refactoring, the following piece of code does not work anymore as in it jumps to the auto, auto case and ignores the Complex, Complex one. I admittedly don't quite understand what the overload is exactly doing, but to me, the two codes look exactly the same, with the exception, that one gets its parameters directly whereas the other one has the parameters defined in the function body itself.

Math_Node Function_Manager::add(const Math_Node& arg){
        Math_Object obj1 = arg.children_ptr()->at(0)->data();
        Math_Object obj2 = arg.children_ptr()->at(1)->data();
        if( std::holds_alternative<Complex>(obj1) ){
            std::cerr << "obj1 is complex\n";
        }
        if( std::holds_alternative<Complex>(obj2) ){
            std::cerr << "obj2 is complex\n";
        }
        return std::visit(overload{
               [](const Complex& a, const Complex& b) -> Math_Object{ 
                   std::cerr << "COMPLEX ADD_\n"; 
                   return add_(a, b); 
               }
             , [](const Matrix& a, const Matrix& b) -> Math_Object{ 
                   std::cerr << "MATRIX ADD_\n"; 
                   return add_(a, b); 
               }
             , [&arg](auto& a, auto& b) -> Math_Node{
                   std::cerr << "NOT FOUND\n"; 
                   return arg;
             }
         }, obj1, obj2);
}

The code prints

obj1 is complex
obj2 is complex
NOT FOUND

This was the working code before refactoring:

Math_Object Function_Manager::add(const Math_Object& arg0, const Math_Object& arg1){
        return
                std::visit(
                        overload{
                                [](const Complex& a, const Complex& b) -> Math_Object{ return add_(a, b); }
                                , [](const Matrix& a, const Matrix& b) -> Math_Object{ return add_(a, b); }
                                , [](auto& a, auto& b) -> Math_Object{
                                    throw std::runtime_error(
                                            ("Unsupported arguments for add: " + to_string(a) + to_string(b)).c_str());
                                }
                        }, arg0, arg1
                          );
    }

The only thing I could come up with was that obj1 and obj2 are not really of the desired types, but the print to std::cerr proves that they are. So why does std::visit not recognise it as such and how can I fix it?

infinitezero
  • 1,610
  • 3
  • 14
  • 29
  • 1
    `auto&` is preferred to `T const&` for a non-const `T`. Try changing `auto&` to `auto const&`. – ecatmur Feb 20 '20 at 17:39
  • Then I get an invalid conversion error. – infinitezero Feb 20 '20 at 17:51
  • @infinitezero Could you please add that error message to your question? I don't see why there should be an error. – walnut Feb 20 '20 at 17:57
  • It's insanely long. Math_Object is an std::variant which contains Complex. Math_Node is a templated Node, where the template is set to Math_Object. By changing the return type of the lambda to Math_Node instead of returning Math_Object, which calls the Math_Node constructor implicitly, I got rid of the error message. – infinitezero Feb 20 '20 at 19:40

1 Answers1

3

In your first example obj1 and obj2 are not const qualified.

In your second example arg0 and arg1 are.

overload simply does overload resolution on the call operators of all the lambdas given to it (assuming it is the usual implementation).


In overload resolution for the first example auto&/auto& is a better match for obj1/obj2 than const Complex&/const Complex&, because the latter requires a qualification conversion to add the const, while auto&/auto& can be deduced to Complex&/Complex&, which does not require a qualification conversion.


This is not the case in the second example, since arg0/arg1 are const, the template argument deduction for auto&/auto& will yield const Complex&/const Complex& and neither this call nor the one taking const Complex&/const Complex& directly will need any conversions.

If two functions have equally good conversion sequences for a call, then the call is disambiguated by a few additional criteria, one of which is that non-template functions are preferred to template functions. The overload taking const Complex&/const Complex& directly is not a template (since it is not a generic lambda) and so it is preferred.


To resolve this, just take the same qualifications in all calls, i.e. take const auto&/const auto& instead of auto&/auto& in the last call or reproduce the second example's overload resolution behavior by passing std::as_const(obj1) and std::as_const(obj2) instead of obj1 and obj2 to std::visit.

walnut
  • 21,629
  • 4
  • 23
  • 59
  • Oh wow, yes it seems obvious now. Thanks. At first I got an invalid conversion error that I could fix by explicit casting! – infinitezero Feb 20 '20 at 17:57
  • 1
    @infinitezero I am not sure what that error would be and why a cast is needed anywhere. Could you add that to your question? Instead of explicit casting or changing the `auto` parameters, you could pass `obj1` and `obj2` via `std::as_const` to `std::visit` to obtain the second examples behavior. I don't see any reason for casts here and explicit casts tend to be dangerous, especially if they cast `const` *away*. – walnut Feb 20 '20 at 17:58