wlav / cppyy

Other
384 stars 38 forks source link

Implicit conversion to `unique_ptr` hijacks overload resolution. #224

Open tmadlener opened 3 months ago

tmadlener commented 3 months ago

This has been reported in a similar form here: https://github.com/root-project/root/issues/15117, but I will try to make this issue stand on it's own and make it reproduce in cppyy version 3.1.2.

TL;DR: When there is an overload with a std::unique_ptr&& that gets precedence over a template function taking the object by value, which should in principle not happen.

A reproducer is the following:

import cppyy

cppyy.cppdef("""
#include <memory>
#include <type_traits>

namespace repro {
struct A {
    int i{42};
};

void foo(std::unique_ptr<A>&& basePtr) {
   std::cout << "via unique_ptr: " << basePtr->i << std::endl;
}

template<typename T, typename = std::enable_if_t<!std::is_lvalue_reference_v<T>>>
void foo(T&& t) {
  std::cout << "via template func: " << t.i << std::endl;
}
} // namespace repro
""")

from cppyy.gbl import repro

c = repro.A(123)
repro.foo(cppyy.gbl.std.move(c))

This will result in

$ python repro.py
via unique_ptr: 123

I would have expected the templated version to be called, since that is the better match in c++.

Note that removing the && from the unique_ptr argument in the first overload makes this work as expected and the template function gets called.

wlav commented 3 months ago

Yes, this is explicitly done this way: implicit conversion is tried before template instantiation for known methods (but not for templated methods). As in TemplateProxy.cxx:

// case 2: select known non-template overload
    result = SelectAndForward(pytmpl, pytmpl->fTI->fNonTemplated, args, nargsf, kwds,
        true /* implicitOkay */, false /* use_targs */, sighash, errors);
    if (result)
        TPPCALL_RETURN;

If memory serves, this choice was made for numeric types.

wlav commented 3 months ago

Setting implicitOkay to false doesn't break any existing tests, but does (unsurprisingly) break the conversion to unique_ptr, meaning that if there is no template to match, to call will then fail altogether. That behavior would then be different from the case where there is no template at all.

I.e., this being a run-time system, whether the first call would work or not would depend on whether it was made before or after loading the template declaration. For sanity's sake, I prefer the current behavior.

tmadlener commented 3 months ago

Thanks for the clarification. It looks like there is no really solution in this case. Given that the recommended way in c++ to do the interface is to use unique_ptr by value (which also requires the move on the calling site), I think the behavior of cppyy is OK. Did I just miss some documentation in that case?