diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx index f5d8e59cf139e..ac702c25769f7 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.cxx +++ b/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). 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() { diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h b/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h index e657999327c11..d6b7adcbe350b 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPInstance.h +++ b/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*); diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx index cf7d27adc84db..d3e472350afe2 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx +++ b/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 holding a Derived must not be accepted where a +// std::unique_ptr is expected: the held smart pointer is still a +// unique_ptr and does not convert to unique_ptr. + if (pyobj->IsSmart() && Cppyy::IsSubtype(pyobj->GetSmartUnderlyingType(), fUnderlyingType)) { para.fValue.fVoidp = ((CPPInstance*)pyobject)->GetSmartObject(); para.fTypeCode = 'V'; return true; diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/ProxyWrappers.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/ProxyWrappers.cxx index 35a76b0552763..6aad4bf43a22e 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/ProxyWrappers.cxx +++ b/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; diff --git a/bindings/pyroot/cppyy/cppyy/test/cpp11features.cxx b/bindings/pyroot/cppyy/cppyy/test/cpp11features.cxx index 7319875c30e4e..51c2f65790c28 100644 --- a/bindings/pyroot/cppyy/cppyy/test/cpp11features.cxx +++ b/bindings/pyroot/cppyy/cppyy/test/cpp11features.cxx @@ -40,6 +40,26 @@ TestSmartPtr create_TestSmartPtr_by_value() { return TestSmartPtr{}; } +std::shared_ptr create_shared_ptr_to_derived() { + return std::shared_ptr(new PubDerivedTestSmartPtr); +} + +std::unique_ptr create_unique_ptr_to_derived() { + return std::unique_ptr(new PubDerivedTestSmartPtr); +} + +std::unique_ptr create_unique_ptr_to_offset_derived() { + return std::unique_ptr(new MultiDerivedTestSmartPtr); +} + +int pass_unique_ptr_to_derived(std::unique_ptr p) { + return p->only_in_derived(); +} + +int pass_shared_ptr_to_derived(std::shared_ptr p) { + return p->only_in_derived(); +} + // for move ctors etc. int TestMoving1::s_move_counter = 0; diff --git a/bindings/pyroot/cppyy/cppyy/test/cpp11features.h b/bindings/pyroot/cppyy/cppyy/test/cpp11features.h index 183ce7082e319..dc5ebb9592b12 100644 --- a/bindings/pyroot/cppyy/cppyy/test/cpp11features.h +++ b/bindings/pyroot/cppyy/cppyy/test/cpp11features.h @@ -36,6 +36,34 @@ int move_unique_ptr_derived(std::unique_ptr&& 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 create_shared_ptr_to_derived(); +std::unique_ptr create_unique_ptr_to_derived(); +std::unique_ptr 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 p); +int pass_shared_ptr_to_derived(std::shared_ptr p); + //=========================================================================== class TestMoving1 { // for move ctors etc. diff --git a/bindings/pyroot/cppyy/cppyy/test/test_cpp11features.py b/bindings/pyroot/cppyy/cppyy/test/test_cpp11features.py index 71c30153dfe02..a83857fb6b51c 100644 --- a/bindings/pyroot/cppyy/cppyy/test/test_cpp11features.py +++ b/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 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__])) diff --git a/tutorials/roofit/roofit/rf617_simulation_based_inference_multidimensional.py b/tutorials/roofit/roofit/rf617_simulation_based_inference_multidimensional.py index 46429c22912ae..fc34d2242e9e9 100644 --- a/tutorials/roofit/roofit/rf617_simulation_based_inference_multidimensional.py +++ b/tutorials/roofit/roofit/rf617_simulation_based_inference_multidimensional.py @@ -261,17 +261,9 @@ def learned_likelihood_ratio(*args): nllr_learned.plotOn(frame1, LineColor="kP6Blue", ShiftToZero=True, Name="learned") -# Declare a helper function in ROOT to dereference unique_ptr -ROOT.gInterpreter.Declare( - """ -RooAbsArg &my_deref(std::unique_ptr const& ptr) { return *ptr; } -""" -) - # Choose normalization set for lhr_calc to plot over norm_set = ROOT.RooArgSet(x_vars) -lhr_calc_final_ptr = ROOT.RooFit.Detail.compileForNormSet(lhr_calc, norm_set) -lhr_calc_final = ROOT.my_deref(lhr_calc_final_ptr) +lhr_calc_final = ROOT.RooFit.Detail.compileForNormSet(lhr_calc, norm_set) lhr_calc_final.recursiveRedirectServers(norm_set) # Plot the likelihood ratio functions