From 6e3bfb08e69c76c080dc83651771688f31b57f31 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 4 Jul 2009 19:30:54 +0200 Subject: [PATCH] OpaqueHolder implemented and passes unit test --- src/lib/opaque-holder.hpp | 215 ++++++++++++++++++++++++------- src/lib/scoped-holder.hpp | 4 +- tests/lib/opaque-holder-test.cpp | 55 ++++++-- 3 files changed, 216 insertions(+), 58 deletions(-) diff --git a/src/lib/opaque-holder.hpp b/src/lib/opaque-holder.hpp index c5e732d07..1a09fc057 100644 --- a/src/lib/opaque-holder.hpp +++ b/src/lib/opaque-holder.hpp @@ -55,12 +55,40 @@ #include "lib/error.hpp" #include "lib/bool-checkable.hpp" - +#include "lib/access-casted.hpp" +#include "lib/util.hpp" namespace lib { using lumiera::error::LUMIERA_ERROR_WRONG_TYPE; + using util::isSameObject; + using util::AccessCasted; + using util::unConst; + + + namespace { // implementation helpers... + + using boost::disable_if; + using boost::is_convertible; + + bool + validitySelfCheck (bool boolConvertible) + { + return boolConvertible; + } + + template + typename disable_if< is_convertible, + bool >::type + validitySelfCheck (X const&) + { + return true; // just pass if this type doesn't provide a validity check... + } + + } + + /** @@ -70,10 +98,14 @@ namespace lib { * may be created empty, which can be checked by a bool test. * The whole compound is copyable if and only if the contained object * is copyable. + * + * For using OpaqueHolder, several \b assumptions need to be fulfilled + * - any instance placed into OpaqueHolder is below the specified maximum size + * - the caller cares for thread safety. No concurrent get calls while in mutation! */ template - < class BA - , size_t siz = sizeof(BA) + < class BA ///< the nominal Base/Interface class for a family of types + , size_t siz = sizeof(BA) ///< maximum storage required for the targets to be held inline > class OpaqueHolder : public BoolCheckable > @@ -83,6 +115,8 @@ namespace lib { struct Buffer { char content_[siz]; + void* ptr() { return &content_; } + virtual ~Buffer() {} virtual bool isValid() const { return false; } virtual bool empty() const { return true; } @@ -92,13 +126,26 @@ namespace lib { { new(targetStorage) Buffer(); } + + virtual BA& + get() const + { + throw lumiera::error::Logic ("get() called on empty Buffer"); + } }; - /** concrete subclass managing a specific kind of contained object */ + /** concrete subclass managing a specific kind of contained object. + * @note invariant: content_ always contains a valid SUB object */ template struct Buff : Buffer { + SUB& + get() const ///< core operation: target is contained within the inline buffer + { + return *reinterpret_cast (unConst(this)->ptr()); + } + ~Buff() { get().~SUB(); @@ -108,16 +155,16 @@ namespace lib { Buff (SUB const& obj) { REQUIRE (siz >= sizeof(SUB)); - new(&content_) SUB (obj); + new(Buffer::ptr()) SUB (obj); } Buff (Buff const& oBuff) { - new(&content_) SUB (oBuff.get()); + new(Buffer::ptr()) SUB (oBuff.get()); } Buff& - operator= (Buff const& ref) + operator= (Buff const& ref) ///< not used currently { if (&ref != this) get() = ref.get(); @@ -140,12 +187,7 @@ namespace lib { bool isValid() const { - UNIMPLEMENTED ("maybe forward bool check to contained object..."); - } - SUB& - get() const - { - return *reinterpret_cast (&content_); + return validitySelfCheck (get()); } }; @@ -153,88 +195,175 @@ namespace lib { enum{ BUFFSIZE = sizeof(Buffer) }; /** embedded buffer actually holding the concrete Buff object, - * which in turn holds and manages the target object */ + * which in turn holds and manages the target object. + * @note Invariant: always contains a valid Buffer subclass */ char storage_[BUFFSIZE]; - typedef OpaqueHolder _ThisType; /////TODO needed? Buffer& buff() { return *reinterpret_cast (&storage_); } + const Buffer& + buff() const + { + return *reinterpret_cast (&storage_); + } + void killBuffer() + { + buff().~Buffer(); + } + + void make_emptyBuff() + { + new(&storage_) Buffer(); + } + + template + void place_inBuff (SUB const& obj) + { + new(&storage_) Buff (obj); + } + + void clone_inBuff (OpaqueHolder const& ref) + { + ref.buff().clone (storage_); + } + public: + ~OpaqueHolder() + { + killBuffer(); + } + + void + clear () + { + killBuffer(); + make_emptyBuff(); + } + + OpaqueHolder() { - new(&storage_) Buffer(); + make_emptyBuff(); } template OpaqueHolder(SUB const& obj) { - new(&storage_) Buff (obj); + place_inBuff (obj); } - ~OpaqueHolder() { clear(); } - - - void - clear () - { - buff().~Holder(); - //////////////////////TODO sufficient? - } - - OpaqueHolder (OpaqueHolder const& ref) { - ref.clone (storage_); + clone_inBuff (ref); } OpaqueHolder& operator= (OpaqueHolder const& ref) { - UNIMPLEMENTED ("copy operation"); + if (!isSameObject (*this, ref)) + { + killBuffer(); + try + { + clone_inBuff (ref); + } + catch (...) + { + make_emptyBuff(); + throw; + } + } return *this; } - + + template + OpaqueHolder& + operator= (SUB const& newContent) + { + if ( !empty() + && !isSameObject (buff().get(), newContent) + ) + { + killBuffer(); + try + { + place_inBuff (newContent); + } + catch (...) + { + make_emptyBuff(); + throw; + } + } + return *this; + } + + + /* === smart-ptr style access === */ BA& - operator* () const // never throws + operator* () const { ASSERT (!empty()); - return (BA&) content_; + return buff().get(); } BA* - operator-> () const // never throws + operator-> () const { ASSERT (!empty()); - return (BA*) &content_; + return &(buff().get()); } template SUB& get() const { - UNIMPLEMENTED ("downcast to concrete type"); -// return (SUB*) &content_; + typedef const Buffer* Iface; + typedef const Buff * Actual; + Iface interface = &buff(); + Actual actual = dynamic_cast (interface); + if (actual) + return actual->get(); + + // second try: maybe we can perform a + // dynamic upcast or direct conversion on the + // concrete target object. But we need to exclude a + // brute force static cast (which might slice or reinterpret) + if (!util::use_static_downcast::value) + { + BA* asBase = &(buff().get()); + SUB* content = AccessCasted::access (asBase); + if (content) + return *content; + } + + throw lumiera::error::Logic ("Attempt to access OpaqueHolder's contents " + "specifying incompatible target type" + , LUMIERA_ERROR_WRONG_TYPE + ); } - bool empty() const + + bool + empty() const { - UNIMPLEMENTED ("check for empty container"); + return buff().empty(); } - bool isValid() const + + bool + isValid() const { - UNIMPLEMENTED ("empty check and forward to contained object"); + return buff().isValid(); } - - }; diff --git a/src/lib/scoped-holder.hpp b/src/lib/scoped-holder.hpp index 95c0909a6..0a1a1d3a8 100644 --- a/src/lib/scoped-holder.hpp +++ b/src/lib/scoped-holder.hpp @@ -195,14 +195,14 @@ namespace lib { TY& - operator* () const // never throws + operator* () const { ASSERT (created_); return (TY&) content_; } TY* - operator-> () const // never throws + operator-> () const { ASSERT (created_); return (TY*) &content_; diff --git a/tests/lib/opaque-holder-test.cpp b/tests/lib/opaque-holder-test.cpp index 3b8e7f94d..3a5d8e5bd 100644 --- a/tests/lib/opaque-holder-test.cpp +++ b/tests/lib/opaque-holder-test.cpp @@ -40,13 +40,14 @@ namespace test{ using util::isnil; using util::for_each; using util::isSameObject; + using lumiera::error::LUMIERA_ERROR_ASSERTION; using std::vector; using std::cout; using std::endl; namespace { // test dummy hierarchy - // Note: no vtable and common storage + // Note: common storage but no vtable long _checksum = 0; @@ -55,11 +56,13 @@ namespace test{ { uint id_; - Base(uint i=0) : id_(i) { _checksum +=id_; } + Base(uint i=0) : id_(i) { _checksum +=id_; } + Base(Base const& o) : id_(o.id_) { _checksum +=id_; } uint getIt() { return id_; } }; + template struct DD : Base { @@ -72,7 +75,7 @@ namespace test{ : DD<7> , BoolCheckable { - uint myVal_; + ulong myVal_; Special (uint val) : myVal_(val) @@ -85,6 +88,12 @@ namespace test{ } }; + + /** maximum additional storage maybe wasted + * due to alignment of the contained object + * within OpaqueHolder's buffer + */ + const size_t ALLIGNMENT = sizeof(size_t); } @@ -97,6 +106,11 @@ namespace test{ * @test use the OpaqueHolder inline buffer to handle a family of classes * through a common interface, without being forced to use heap storage * or a custom allocator. + * + * @todo this test doesn't cover automatic conversions and conversions using + * RTTI from the target objects, while OpaqueHolder.tempate get() would + * allow for such conversions. This is similar to Ticket #141, and actually + * based on the same code as variant.hpp (access-casted.hpp) */ class OpaqueHolder_test : public Test { @@ -149,17 +163,17 @@ namespace test{ typedef DD<5> D5; D3 d3 (oo.get() ); ASSERT (3 == oo->getIt()); - ASSERT (isSameObject (d3, *oo)); + ASSERT (!isSameObject (d3, *oo)); VERIFY_ERROR (WRONG_TYPE, oo.get() ); - oo = D5(); + oo = D5(); // direct assignment of target into Buffer ASSERT (oo); ASSERT (5 == oo->getIt()); VERIFY_ERROR (WRONG_TYPE, oo.get() ); D5 &rd5 (oo.get()); ASSERT (isSameObject (rd5, *oo)); - + // verify that self-assignment is properly detected... oo = oo; ASSERT (oo); @@ -179,22 +193,37 @@ namespace test{ ASSERT (!o1); Opaque o2 (d3); - ASSERT (!isSameObject (d3, *oo)); - ASSERT (3 == oo->getIt()); + ASSERT (!isSameObject (d3, *o2)); + ASSERT (3 == o2->getIt()); - ASSERT (sizeof(Opaque) == sizeof(Base) + sizeof(void*)); + ASSERT (sizeof(Opaque) <= sizeof(Base) + sizeof(void*) + ALLIGNMENT); } + /** @test OpaqueHolder with additional storage for subclass. + * When a subclass requires more storage than the base class or + * Interface, we need to create a custom OpaqueHolder, specifying the + * actually necessary storage. Such a custom OpaqueHolder behaves exactly + * like the standard variant, but there is protection against accidentally + * using a standard variant to hold an instance of the larger subclass. + * + * @test Moreover, if the concrete class has a custom operator bool(), it + * will be invoked automatically from OpaqueHolder's operator bool() + * + */ void checkSpecialSubclass () { - // define a "special" OpaqueHolder with enough storage to hold a Special typedef OpaqueHolder SpecialOpaque; + cout << showSizeof() << endl; + cout << showSizeof() << endl; + cout << showSizeof() << endl; + cout << showSizeof() << endl; + ASSERT (sizeof(Special) > sizeof(Base)); ASSERT (sizeof(SpecialOpaque) > sizeof(Opaque)); - ASSERT (sizeof(SpecialOpaque) == sizeof(Special) + sizeof(void*)); + ASSERT (sizeof(SpecialOpaque) <= sizeof(Special) + sizeof(void*) + ALLIGNMENT); Special s1 (6); Special s2 (3); @@ -215,8 +244,8 @@ namespace test{ ASSERT (!isnil(ospe2)); ASSERT (7 == ospe1->getIt()); - ASSERT (5 == ospe1.get().myVal_); - ASSERT (6 == ospe2.get().myVal_); + ASSERT (6 == ospe1.get().myVal_); + ASSERT (3 == ospe2.get().myVal_); ospe1 = DD<5>(); // but can be reassigned like any normal Opaque ASSERT (ospe1);