From 6af8d32078a1aef51c3cc46b8b4dbdb66929014e Mon Sep 17 00:00:00 2001 From: Thomas Geymayer Date: Sun, 20 Jul 2014 19:50:53 +0200 Subject: [PATCH] cppbind: fix Ghost casting/storage in polymorphic class hierarchies. --- simgear/nasal/cppbind/CMakeLists.txt | 2 + simgear/nasal/cppbind/Ghost.cxx | 11 +- simgear/nasal/cppbind/Ghost.hxx | 201 ++++++++++++------- simgear/nasal/cppbind/NasalContext.cxx | 49 +++++ simgear/nasal/cppbind/NasalContext.hxx | 43 ++++ simgear/nasal/cppbind/cppbind_test.cxx | 4 - simgear/nasal/cppbind/cppbind_test_ghost.cxx | 48 ++++- 7 files changed, 273 insertions(+), 85 deletions(-) create mode 100644 simgear/nasal/cppbind/NasalContext.cxx create mode 100644 simgear/nasal/cppbind/NasalContext.hxx diff --git a/simgear/nasal/cppbind/CMakeLists.txt b/simgear/nasal/cppbind/CMakeLists.txt index 4cb1c5ae..de6eece6 100644 --- a/simgear/nasal/cppbind/CMakeLists.txt +++ b/simgear/nasal/cppbind/CMakeLists.txt @@ -3,6 +3,7 @@ include (SimGearComponent) set(HEADERS Ghost.hxx NasalCallContext.hxx + NasalContext.hxx NasalHash.hxx NasalObject.hxx NasalObjectHolder.hxx @@ -21,6 +22,7 @@ set(DETAIL_HEADERS set(SOURCES Ghost.cxx + NasalContext.cxx NasalHash.cxx NasalString.cxx NasalObject.cxx diff --git a/simgear/nasal/cppbind/Ghost.cxx b/simgear/nasal/cppbind/Ghost.cxx index dcfb55af..a89463cf 100644 --- a/simgear/nasal/cppbind/Ghost.cxx +++ b/simgear/nasal/cppbind/Ghost.cxx @@ -33,7 +33,7 @@ namespace nasal } //-------------------------------------------------------------------------- - bool GhostMetadata::isBaseOf(naGhostType* ghost_type, bool& is_weak) const + bool GhostMetadata::isInstance(naGhostType* ghost_type, bool& is_weak) const { if( ghost_type == _ghost_type_strong_ptr ) { @@ -47,14 +47,6 @@ namespace nasal return true; } - for( DerivedList::const_iterator derived = _derived_classes.begin(); - derived != _derived_classes.end(); - ++derived ) - { - if( (*derived)->isBaseOf(ghost_type, is_weak) ) - return true; - } - return false; } @@ -74,7 +66,6 @@ namespace nasal void GhostMetadata::addDerived(const GhostMetadata* derived) { assert(derived); - _derived_classes.push_back(derived); SG_LOG ( diff --git a/simgear/nasal/cppbind/Ghost.hxx b/simgear/nasal/cppbind/Ghost.hxx index 6dcb5e15..1a02e367 100644 --- a/simgear/nasal/cppbind/Ghost.hxx +++ b/simgear/nasal/cppbind/Ghost.hxx @@ -107,17 +107,14 @@ namespace nasal */ void addNasalBase(const naRef& parent); - bool isBaseOf(naGhostType* ghost_type, bool& is_weak) const; + bool isInstance(naGhostType* ghost_type, bool& is_weak) const; protected: - typedef std::vector DerivedList; - const std::string _name_strong, _name_weak; const naGhostType *_ghost_type_strong_ptr, *_ghost_type_weak_ptr; - DerivedList _derived_classes; std::vector _parents; GhostMetadata( const std::string& name, @@ -307,7 +304,7 @@ namespace nasal // Keep reference for duration of call to prevent expiring // TODO not needed for strong referenced ghost - strong_ref ref = fromNasal(c, me); + strong_ref ref = fromNasal(c, me); if( !ref ) { naGhostType* ghost_type = naGhost_type(me); @@ -416,10 +413,13 @@ namespace nasal boost::is_base_of::value )); + typedef typename BaseGhost::strong_ref base_ref; + BaseGhost* base = BaseGhost::getSingletonPtr(); base->addDerived( this, - SG_GET_TEMPLATE_MEMBER(Ghost, getTypeFor) + SG_GET_TEMPLATE_MEMBER(Ghost, toNasal), + SG_GET_TEMPLATE_MEMBER(Ghost, fromNasalWithCast) ); // Replace any getter that is not available in the current class. @@ -809,7 +809,7 @@ namespace nasal makeGhost(naContext c, RefType const& ref_ptr) { strong_ref ref(ref_ptr); - raw_type* ptr = get_pointer(ref_ptr); + raw_type* ptr = get_pointer(ref); if( !ptr ) return naNil(); @@ -817,15 +817,11 @@ namespace nasal // will then be hold be a new shared pointer. We therefore have to // check for the dynamic type of the object as it might differ from // the passed static type. - naGhostType* ghost_type = - getTypeFor(ptr, shared_ptr_traits::is_strong::value); - - if( !ghost_type ) - return naNil(); - - return naNewGhost2( c, - ghost_type, - shared_ptr_storage::ref(ref_ptr) ); + return toNasal( + c, + ref, + shared_ptr_traits::is_strong::value + ); } /** @@ -833,25 +829,36 @@ namespace nasal * Nasal objects has to be derived class of the target class (Either * derived in C++ or in Nasal using a 'parents' vector) */ - template - static Type fromNasal(naContext c, naRef me) + static strong_ref fromNasal(naContext c, naRef me) { - bool is_weak = false; - - // Check if it's a ghost and if it can be converted - if( isBaseOf(naGhost_type(me), is_weak) ) + naGhostType* ghost_type = naGhost_type(me); + if( ghost_type ) { - void* ghost = naGhost_ptr(me); - return is_weak ? getPtr(ghost) - : getPtr(ghost); - } + // Check if we got an instance of this class + bool is_weak = false; + if( isInstance(ghost_type, is_weak) ) + { + return is_weak ? getPtr(naGhost_ptr(me)) + : getPtr(naGhost_ptr(me)); + } - // Now if it is derived from a ghost (hash with ghost in parent vector) + // otherwise try the derived classes + for( typename DerivedList::reverse_iterator + derived = getSingletonPtr()->_derived_types.rbegin(); + derived != getSingletonPtr()->_derived_types.rend(); + ++derived ) + { + strong_ref ref = (derived->from_nasal)(c, me); + + if( get_pointer(ref) ) + return ref; + } + } else if( naIsHash(me) ) { naRef na_parents = naHash_cget(me, const_cast("parents")); if( !naIsVector(na_parents) ) - return Type(); + return strong_ref(); typedef std::vector naRefs; naRefs parents = from_nasal(c, na_parents); @@ -859,19 +866,13 @@ namespace nasal parent != parents.end(); ++parent ) { - Type ptr = fromNasal(c, *parent); - if( get_pointer(ptr) ) - return ptr; + strong_ref ref = fromNasal(c, *parent); + if( get_pointer(ref) ) + return ref; } } - return Type(); - } - - static bool isBaseOf(naRef obj) - { - bool is_weak; - return isBaseOf(naGhost_type(obj), is_weak); + return strong_ref(); } private: @@ -882,21 +883,30 @@ namespace nasal static naGhostType _ghost_type_strong, //!< Stored as shared pointer _ghost_type_weak; //!< Stored as weak shared pointer - typedef naGhostType* (*type_checker_t)(const raw_type*, bool); - typedef std::vector DerivedList; + typedef naRef (*to_nasal_t)(naContext, const strong_ref&, bool); + typedef strong_ref (*from_nasal_t)(naContext, naRef); + struct DerivedInfo + { + to_nasal_t to_nasal; + from_nasal_t from_nasal; + + DerivedInfo( to_nasal_t to_nasal_func, + from_nasal_t from_nasal_func ): + to_nasal(to_nasal_func), + from_nasal(from_nasal_func) + {} + }; + + typedef std::vector DerivedList; DerivedList _derived_types; - static bool isBaseOf(naGhostType* ghost_type, bool& is_weak) + static bool isInstance(naGhostType* ghost_type, bool& is_weak) { if( !ghost_type || !getSingletonPtr() ) return false; - return getSingletonPtr()->GhostMetadata::isBaseOf(ghost_type, is_weak); - } - - static bool isBaseOf(naRef obj, bool& is_weak) - { - return isBaseOf(naGhost_type(obj), is_weak); + return getSingletonPtr()->GhostMetadata::isInstance( ghost_type, + is_weak ); } template @@ -945,28 +955,33 @@ namespace nasal } void addDerived( const internal::GhostMetadata* derived_meta, - type_checker_t derived_type_checker ) + to_nasal_t to_nasal_func, + from_nasal_t from_nasal_func ) { GhostMetadata::addDerived(derived_meta); - _derived_types.push_back(derived_type_checker); + _derived_types.push_back(DerivedInfo(to_nasal_func, from_nasal_func)); } template static typename boost::enable_if < boost::is_polymorphic, - naGhostType* + naRef >::type - getTypeFor(const typename BaseGhost::raw_type* base, bool strong) + toNasal( naContext c, + const typename BaseGhost::strong_ref& base_ref, + bool strong ) { + typename BaseGhost::raw_type* ptr = get_pointer(base_ref); + // Check first if passed pointer can by converted to instance of class // this ghost wraps. if( !boost::is_same < typename BaseGhost::raw_type, typename Ghost::raw_type >::value - && dynamic_cast(base) != base ) - return 0; + && dynamic_cast(ptr) != ptr ) + return naNil(); if( !getSingletonPtr() ) { @@ -976,45 +991,52 @@ namespace nasal SG_INFO, "Ghost::getTypeFor: can not get type for unregistered ghost" ); - return 0; + return naNil(); } + strong_ref ref = + static_pointer_cast(base_ref); + // Now check if we can further downcast to one of our derived classes. for( typename DerivedList::reverse_iterator derived = getSingletonPtr()->_derived_types.rbegin(); derived != getSingletonPtr()->_derived_types.rend(); ++derived ) { - naGhostType* ghost_type = - (*derived)( - static_cast(base), - strong - ); + naRef ghost = (derived->to_nasal)(c, ref, strong); - if( ghost_type ) - return ghost_type; + if( !naIsNil(ghost) ) + return ghost; } // If base is not an instance of any derived class, this class has to // be the dynamic type. return strong - ? &_ghost_type_strong - : &_ghost_type_weak; + ? create(c, ref) + : create(c, ref); } template static typename boost::disable_if < boost::is_polymorphic, - naGhostType* + naRef >::type - getTypeFor(const typename BaseGhost::raw_type* base, bool strong) + toNasal( naContext c, + const typename BaseGhost::strong_ref& ref, + bool strong ) { // For non polymorphic classes there is no possibility to get the actual // dynamic type, therefore we can only use its static type. return strong - ? &BaseGhost::_ghost_type_strong - : &BaseGhost::_ghost_type_weak; + ? create(c, ref) + : create(c, ref); + } + + template + static Type fromNasalWithCast(naContext c, naRef me) + { + return Type(fromNasal(c, me)); } static Ghost* getSingletonPtr() @@ -1176,6 +1198,45 @@ namespace nasal return instance; } + template + static + typename boost::enable_if_c< + !is_weak, + naRef + >::type + create(naContext c, const strong_ref& ref_ptr) + { + typedef shared_ptr_storage storage_type; + return naNewGhost2( c, + &Ghost::_ghost_type_strong, + storage_type::ref(ref_ptr) ); + } + + template + static + typename boost::enable_if_c< + is_weak && supports_weak_ref::value, + naRef + >::type + create(naContext c, const strong_ref& ref_ptr) + { + typedef shared_ptr_storage storage_type; + return naNewGhost2( c, + &Ghost::_ghost_type_weak, + storage_type::ref(ref_ptr) ); + } + + template + static + typename boost::enable_if_c< + is_weak && !supports_weak_ref::value, + naRef + >::type + create(naContext, const strong_ref&) + { + return naNil(); + } + template static void destroy(void *ptr) { @@ -1354,7 +1415,7 @@ typename boost::enable_if< from_nasal_helper(naContext c, naRef ref, const T*) { typedef typename nasal::shared_ptr_traits::strong_ref strong_ref; - return nasal::Ghost::template fromNasal(c, ref); + return T(nasal::Ghost::fromNasal(c, ref)); } /** @@ -1389,7 +1450,7 @@ typename boost::enable_if_c< from_nasal_helper(naContext c, naRef ref, const T*) { typedef SGSharedPtr::type> TypeRef; - return nasal::Ghost::template fromNasal(c, ref); + return T(nasal::Ghost::fromNasal(c, ref)); } /** @@ -1416,7 +1477,7 @@ typename boost::enable_if< from_nasal_helper(naContext c, naRef ref, const T*) { typedef osg::ref_ptr::type> TypeRef; - return nasal::Ghost::template fromNasal(c, ref); + return T(nasal::Ghost::fromNasal(c, ref)); } #endif /* SG_NASAL_GHOST_HXX_ */ diff --git a/simgear/nasal/cppbind/NasalContext.cxx b/simgear/nasal/cppbind/NasalContext.cxx new file mode 100644 index 00000000..d2a82d88 --- /dev/null +++ b/simgear/nasal/cppbind/NasalContext.cxx @@ -0,0 +1,49 @@ +// Manage lifetime and encapsulate a Nasal context +// +// Copyright (C) 2014 Thomas Geymayer +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU Library General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + +#include "NasalContext.hxx" + +namespace nasal +{ + + //---------------------------------------------------------------------------- + Context::Context(): + _ctx(naNewContext()) + { + + } + + //---------------------------------------------------------------------------- + Context::~Context() + { + naFreeContext(_ctx); + } + + //---------------------------------------------------------------------------- + Context::operator naContext() + { + return _ctx; + } + + //---------------------------------------------------------------------------- + Hash Context::newHash() + { + return Hash(_ctx); + } + +} // namespace nasal diff --git a/simgear/nasal/cppbind/NasalContext.hxx b/simgear/nasal/cppbind/NasalContext.hxx new file mode 100644 index 00000000..00f84674 --- /dev/null +++ b/simgear/nasal/cppbind/NasalContext.hxx @@ -0,0 +1,43 @@ +///@file Manage lifetime and encapsulate a Nasal context +// +// Copyright (C) 2014 Thomas Geymayer +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU Library General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + +#ifndef SG_NASAL_CONTEXT_HXX_ +#define SG_NASAL_CONTEXT_HXX_ + +#include "NasalHash.hxx" + +namespace nasal +{ + + class Context + { + public: + Context(); + ~Context(); + + operator naContext(); + + Hash newHash(); + + protected: + naContext _ctx; + }; + +} // namespace nasal + +#endif /* SG_NASAL_CONTEXT_HXX_ */ diff --git a/simgear/nasal/cppbind/cppbind_test.cxx b/simgear/nasal/cppbind/cppbind_test.cxx index 154aa1b1..c3b794c8 100644 --- a/simgear/nasal/cppbind/cppbind_test.cxx +++ b/simgear/nasal/cppbind/cppbind_test.cxx @@ -322,10 +322,6 @@ int main(int argc, char* argv[]) == ref_based.get() ); VERIFY( from_nasal(c, na_ref_based) == ref_based ); - VERIFY( Ghost::isBaseOf(derived) ); - VERIFY( Ghost::isBaseOf(derived) ); - VERIFY( Ghost::isBaseOf(derived) ); - VERIFY( from_nasal(c, derived) == d3 ); VERIFY( from_nasal(c, derived) != d2 ); VERIFY( from_nasal(c, derived) diff --git a/simgear/nasal/cppbind/cppbind_test_ghost.cxx b/simgear/nasal/cppbind/cppbind_test_ghost.cxx index 67a265bd..50d42a57 100644 --- a/simgear/nasal/cppbind/cppbind_test_ghost.cxx +++ b/simgear/nasal/cppbind/cppbind_test_ghost.cxx @@ -2,6 +2,8 @@ #include #include "Ghost.hxx" +#include "NasalContext.hxx" + #include #include @@ -49,14 +51,19 @@ BOOST_STATIC_ASSERT(( nasal::supports_weak_ref::value)); BOOST_STATIC_ASSERT(( nasal::supports_weak_ref::value)); BOOST_STATIC_ASSERT((!nasal::supports_weak_ref::value)); -BOOST_AUTO_TEST_CASE( ghost_weak_strong_nasal_conversion ) +static void setupGhosts() { nasal::Ghost::init("Base1"); nasal::Ghost::init("Base2"); nasal::Ghost::init("Derived") .bases() .bases(); +} +//------------------------------------------------------------------------------ +BOOST_AUTO_TEST_CASE( ghost_weak_strong_nasal_conversion ) +{ + setupGhosts(); naContext c = naNewContext(); DerivedPtr d = new Derived(); @@ -91,6 +98,45 @@ BOOST_AUTO_TEST_CASE( ghost_weak_strong_nasal_conversion ) BOOST_REQUIRE( !weak.lock() ); } +//------------------------------------------------------------------------------ +BOOST_AUTO_TEST_CASE( ghost_casting_storage ) +{ + setupGhosts(); + nasal::Context c; + + // Check converting to and from every class in the hierarchy for an instance + // of the leaf class + DerivedPtr d = new Derived(); + + naRef na_d = nasal::to_nasal(c, d), + na_b1 = nasal::to_nasal(c, Base1Ptr(d)), + na_b2 = nasal::to_nasal(c, Base2Ptr(d)); + + Derived *d0 = nasal::from_nasal(c, na_d), + *d1 = nasal::from_nasal(c, na_b1), + *d2 = nasal::from_nasal(c, na_b2); + + BOOST_CHECK_EQUAL(d0, d.get()); + BOOST_CHECK_EQUAL(d1, d.get()); + BOOST_CHECK_EQUAL(d2, d.get()); + + Base1 *b1 = nasal::from_nasal(c, na_b1); + BOOST_CHECK_EQUAL(b1, static_cast(d.get())); + + Base2 *b2 = nasal::from_nasal(c, na_b2); + BOOST_CHECK_EQUAL(b2, static_cast(d.get())); + + // Check converting from base class instance to derived classes is not + // possible + Base1Ptr b1_ref = new Base1(); + na_b1 = nasal::to_nasal(c, b1_ref); + + BOOST_CHECK_EQUAL(nasal::from_nasal(c, na_b1), b1_ref.get()); + BOOST_CHECK(!nasal::from_nasal(c, na_b1)); + BOOST_CHECK(!nasal::from_nasal(c, na_b1)); +} + +//------------------------------------------------------------------------------ #define CHECK_PTR_STORAGE_TRAIT_TYPE(ptr_t, storage)\ BOOST_STATIC_ASSERT((boost::is_same<\ nasal::shared_ptr_storage::storage_type,\ -- 2.39.5