[Feature Request]: Full member function support in `BestOverloadFunctionMatch`

[jeaye]

Description of new feature

I've found that BestOverloadFunctionMatch doesn't select any member function if one is const. It also doesn't support overloading based on & or &&.

struct bar
{
  int foo(int i)
  {
    return i;
  }

  int foo(int i) const
  {
    return i + 1;
  }
};

If the second foo is commented out, I can match the first member fn and call it no problem. But when both are present, neither matches.

This also brings up the important point that BestOverloadFunctionMatch doesn't take into account the constness or the value category of the invoking object. So matching const member fns or matching & or && member fns cannot be done without specifying more information to overload resolution.

I think we should be able to solve this by having the invoking object as the first argument and then resolve on that. Right now, BestOverloadFunctionMatch actually expects that it's not present. So, in my example above, the only argument would be one int.

If instead we need to provide a bar const& and int then we can match the overload on constness and value category of the invoking object.

@Vipul-Cariappa for vis.

[Priyanshu3820]

hey @jeaye, mind checking my PR?

[jeaye]

hey @jeaye, mind checking my PR?

Hi, @Priyanshu3820. The jank fork has an implementation of this which supports const member fns, but not ref-qualified matching, which is the same behavior your PR targets. It also builds upon that to support argument scopes, which are necessary for some more complex cases of overload resolution. For example std::cout << std::endl;. It's here: https://github.com/jank-lang/CppInterOp/blob/main/lib/Interpreter/CppInterOp.cpp#L1417

Aside from those, the behavior in your PR defaults to selecting the first candidate when there are no viable functions, based on what I can see. That is not correct behavior for C++.

[Priyanshu3820]

Hi, @Priyanshu3820. The jank fork has an implementation of this which supports const member fns, but not ref-qualified matching, which is the same behavior your PR targets. It also builds upon that to support argument scopes, which are necessary for some more complex cases of overload resolution. For example std::cout << std::endl;. It's here: https://github.com/jank-lang/CppInterOp/blob/main/lib/Interpreter/CppInterOp.cpp#L1417

Aside from those, the behavior in your PR defaults to selecting the first candidate when there are no viable functions, based on what I can see. That is not correct behavior for C++.

Thanks for taking time, I realize what I did wrong there, I was a bit confused about the handling of static functions but now I see they
are treated like normal functions when there's no invoking object. I removed that fallback and added some more tests including the tests for ref-qualified matching. Now i'll port the support for argument scopes from jank fork.

[Priyanshu3820]

I'll port the support for argument scopes in a separate PR as there's already too much for a single PR.