[cppyy] Auto-downcast objects returned through smart pointers

Returning an object by raw pointer already triggers an automatic
downcast to its actual (most derived) class, but returning it through a
smart pointer (`std::unique_ptr`, `std::shared_ptr`) did not: the object
was bound as the declared underlying type, so derived-class members were
not accessible.

This became more and more of a nuisance as smart pointers become more
common in C++ interface.

Therefore, this commit implements automatic downcasting also for returned
smart pointers.

Closes #16210.

Author: guitargeek

bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx

@@ -187,6 +187,17 @@ Cppyy::TCppType_t CPyCppyy::CPPInstance::GetSmartIsA() const
     return SMART_TYPE(this);
 }
 
+//----------------------------------------------------------------------------
+Cppyy::TCppType_t CPyCppyy::CPPInstance::GetSmartUnderlyingType() const
+{
+// The declared underlying type of the embedded smart pointer (e.g. 'Base' for
+// a std::unique_ptr<Base>). This is independent of any auto-down-cast applied
+// to the dereferenced object, and so is what must be used to decide whether the
+// smart pointer can be passed to a function expecting a particular smart type.
+    if (!IsSmart()) return (Cppyy::TCppType_t)0;
+    return SMART_CLS(this)->fUnderlyingType;
+}
+
 //----------------------------------------------------------------------------
 CPyCppyy::CI_DatamemberCache_t& CPyCppyy::CPPInstance::GetDatamemberCache()
 {

bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h

@@ -77,6 +77,7 @@ class CPPInstance {
     void SetSmart(PyObject* smart_type);
     void* GetSmartObject() { return GetObjectRaw(); }
     Cppyy::TCppType_t GetSmartIsA() const;
+    Cppyy::TCppType_t GetSmartUnderlyingType() const;
 
 // cross-inheritance dispatch
     void SetDispatchPtr(void*);

bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx

@@ -3066,7 +3066,12 @@ bool CPyCppyy::SmartPtrConverter::SetArg(
     }
 
 // final option, try mapping pointer types held (TODO: do not allow for non-const ref)
-    if (pyobj->IsSmart() && Cppyy::IsSubtype(oisa, fUnderlyingType)) {
+// Note: this must be decided on the smart pointer's *declared* underlying type, not
+// on the (possibly auto-down-cast) type of the dereferenced object. A
+// std::unique_ptr<Base> holding a Derived must not be accepted where a
+// std::unique_ptr<Derived> is expected: the held smart pointer is still a
+// unique_ptr<Base> and does not convert to unique_ptr<Derived>.
+    if (pyobj->IsSmart() && Cppyy::IsSubtype(pyobj->GetSmartUnderlyingType(), fUnderlyingType)) {
         para.fValue.fVoidp = ((CPPInstance*)pyobject)->GetSmartObject();
         para.fTypeCode = 'V';
         return true;

bindings/pyroot/cppyy/CPyCppyy/src/ProxyWrappers.cxx

@@ -842,7 +842,31 @@ PyObject* CPyCppyy::BindCppObjectNoCast(Cppyy::TCppObject_t address,
     PyObject* smart_type = (!(flags & CPPInstance::kNoWrapConv) && \
                             (((CPPClass*)pyclass)->fFlags & CPPScope::kIsSmart)) ? pyclass : nullptr;
     if (smart_type) {
-        pyclass = CreateScopeProxy(((CPPSmartClass*)smart_type)->fUnderlyingType);
+        Cppyy::TCppType_t underlying = ((CPPSmartClass*)smart_type)->fUnderlyingType;
+
+    // Down-cast the underlying object to its actual (most derived) class, just
+    // as BindCppObject does for raw pointers. Two conditions must hold:
+    //  * the reported actual class must really be a subtype of the declared one.
+    //    Cppyy::GetActualClass() can return a *base* class (e.g. when the actual
+    //    class has no dictionary of its own and inherits IsA() from a base with
+    //    ClassDef, ROOT reports that base) -- such an up-cast must be rejected.
+    //  * the cast must require no pointer adjustment, because the smart pointer's
+    //    dereferencer always yields a pointer to the underlying (declared) class,
+    //    so a non-zero offset can not be applied consistently on later member
+    //    access (e.g. with multiple inheritance).
+        if (address && !isRef) {
+            void* deref = Cppyy::CallR(
+                ((CPPSmartClass*)smart_type)->fDereferencer, address, 0, nullptr);
+            if (deref) {
+                Cppyy::TCppType_t clActual = Cppyy::GetActualClass(underlying, deref);
+                if (clActual && clActual != underlying &&
+                        Cppyy::IsSubtype(clActual, underlying) &&
+                        Cppyy::GetBaseOffset(clActual, underlying, deref, -1 /* down-cast */) == 0)
+                    underlying = clActual;
+            }
+        }
+
+        pyclass = CreateScopeProxy(underlying);
         if (!pyclass) {
         // simply restore and expose as the actual smart pointer class
             pyclass = smart_type;

bindings/pyroot/cppyy/cppyy/test/cpp11features.cxx

@@ -40,6 +40,26 @@ TestSmartPtr create_TestSmartPtr_by_value() {
     return TestSmartPtr{};
 }
 
+std::shared_ptr<TestSmartPtr> create_shared_ptr_to_derived() {
+    return std::shared_ptr<TestSmartPtr>(new PubDerivedTestSmartPtr);
+}
+
+std::unique_ptr<TestSmartPtr> create_unique_ptr_to_derived() {
+    return std::unique_ptr<TestSmartPtr>(new PubDerivedTestSmartPtr);
+}
+
+std::unique_ptr<TestSmartPtrIface> create_unique_ptr_to_offset_derived() {
+    return std::unique_ptr<TestSmartPtrIface>(new MultiDerivedTestSmartPtr);
+}
+
+int pass_unique_ptr_to_derived(std::unique_ptr<PubDerivedTestSmartPtr> p) {
+    return p->only_in_derived();
+}
+
+int pass_shared_ptr_to_derived(std::shared_ptr<PubDerivedTestSmartPtr> p) {
+    return p->only_in_derived();
+}
+
 
 // for move ctors etc.
 int TestMoving1::s_move_counter = 0;

bindings/pyroot/cppyy/cppyy/test/cpp11features.h

@@ -36,6 +36,34 @@ int move_unique_ptr_derived(std::unique_ptr<DerivedTestSmartPtr>&& p);
 
 TestSmartPtr create_TestSmartPtr_by_value();
 
+// for auto-downcast of objects returned through a smart pointer
+class PubDerivedTestSmartPtr : public TestSmartPtr {
+public:
+    int only_in_derived() { return 27; }
+};
+
+// second base so that the cross-cast to the most derived type needs a
+// non-zero pointer adjustment, which the smart pointer's dereferencer can
+// not apply consistently (so no down-cast should happen in that case)
+class TestSmartPtrIface {
+public:
+    virtual ~TestSmartPtrIface() {}
+    long m_pad = 0;
+    int only_in_iface() { return 37; }
+};
+
+class MultiDerivedTestSmartPtr : public PubDerivedTestSmartPtr, public TestSmartPtrIface {
+};
+
+std::shared_ptr<TestSmartPtr> create_shared_ptr_to_derived();
+std::unique_ptr<TestSmartPtr> create_unique_ptr_to_derived();
+std::unique_ptr<TestSmartPtrIface> create_unique_ptr_to_offset_derived();
+
+// sinks expecting a smart pointer to the *derived* type; a base-class smart
+// pointer (even when its object was auto-down-cast) must not be accepted here
+int pass_unique_ptr_to_derived(std::unique_ptr<PubDerivedTestSmartPtr> p);
+int pass_shared_ptr_to_derived(std::shared_ptr<PubDerivedTestSmartPtr> p);
+
 
 //===========================================================================
 class TestMoving1 {          // for move ctors etc.

bindings/pyroot/cppyy/cppyy/test/test_cpp11features.py

@@ -593,6 +593,45 @@ def test20_tuple_element(self):
 
         cppyy.gbl.std.tuple_element[1, ATuple].type
 
+    def test21_smart_ptr_downcast(self):
+        """Object returned through a smart pointer is auto-downcast"""
+
+        import cppyy
+
+        from cppyy.gbl import TestSmartPtr, PubDerivedTestSmartPtr, TestSmartPtrIface
+        from cppyy.gbl import create_unique_ptr_instance, create_shared_ptr_to_derived
+        from cppyy.gbl import create_unique_ptr_to_derived, create_unique_ptr_to_offset_derived
+        from cppyy.gbl import pass_shared_ptr, pass_unique_ptr_to_derived, pass_shared_ptr_to_derived
+
+        # unique_ptr<Base> holding a Derived comes back as Derived, with the
+        # derived-only method callable, just like a raw pointer return
+        for cf in [create_unique_ptr_to_derived, create_shared_ptr_to_derived]:
+            obj = cf()
+            assert type(obj) == PubDerivedTestSmartPtr
+            assert obj.only_in_derived() == 27
+            assert obj.__smartptr__()      # smart-pointer semantics preserved
+
+        # an object that really is of the declared type stays that type
+        obj = create_unique_ptr_instance()
+        assert type(obj) == TestSmartPtr
+
+        # the most derived type sits at a non-zero offset from the declared
+        # interface, which the dereferencer can not apply: stay the declared
+        # type and keep behaving correctly
+        obj = create_unique_ptr_to_offset_derived()
+        assert type(obj) == TestSmartPtrIface
+        assert obj.only_in_iface() == 37
+
+        # the auto-down-cast must not enable C++-invalid conversions: the proxy
+        # still embeds a smart pointer to the *base* type, which does not convert
+        # to a smart pointer to the derived type (no implicit down-conversion of
+        # smart pointers in C++), so passing it to such a sink must be rejected
+        raises(TypeError, pass_unique_ptr_to_derived, create_unique_ptr_to_derived())
+        raises(TypeError, pass_shared_ptr_to_derived, create_shared_ptr_to_derived())
+
+        # passing it where the matching base smart pointer is expected still works
+        assert pass_shared_ptr(create_shared_ptr_to_derived()) == 17
+
 
 if __name__ == "__main__":
     exit(pytest.main(args=['-sv', '-ra', __file__]))