diff --git a/src/gui/interact/state-map-grouping-storage.hpp b/src/gui/interact/state-map-grouping-storage.hpp index c4d1eb3d7..138144ee3 100644 --- a/src/gui/interact/state-map-grouping-storage.hpp +++ b/src/gui/interact/state-map-grouping-storage.hpp @@ -39,6 +39,7 @@ #include "lib/error.hpp" #include "lib/idi/entry-id.hpp" #include "lib/diff/gen-node.hpp" +#include "lib/util.hpp" #include #include @@ -52,6 +53,7 @@ namespace interact { using lib::idi::BareEntryID; using lib::diff::GenNode; using lib::diff::Ref; + using util::unConst; using std::string; @@ -117,7 +119,9 @@ namespace interact { void record (BareEntryID const& elementID, GenNode const& stateMark) { - elmTable_[elementID].emplace (stateMark); + auto res = elmTable_[elementID].emplace (stateMark); + if (not res.second) + unConst(*res.first) = stateMark; // update existing contents } diff --git a/src/lib/diff/gen-node.hpp b/src/lib/diff/gen-node.hpp index 7672f11d2..b883861ce 100644 --- a/src/lib/diff/gen-node.hpp +++ b/src/lib/diff/gen-node.hpp @@ -269,8 +269,44 @@ namespace diff{ GenNode(Ref & r); GenNode(Ref && r); - GenNode& operator= (GenNode const&) =default; - GenNode& operator= (GenNode&&) =default; + /** copy assignment + * @remarks we need to define our own version here for sake of sanity. + * The reason is that we use inline storage (embedded within lib::Variant) + * and that we deliberately _erase_ the actual type of data stored inline. + * Because we still do want copy assignment, in case the payload data + * supports this, we use a "virtual copy operator", where in the end + * the storage buffer within lib::Variant has to decide if assignment + * is possible. Only data with the same type may be assigned and we + * prevent change of the (implicit) data type through assignment. + * This check might throw, and for that reason we're better off + * to perform the _data assignment_ first. The probability for + * EntryID assignment to fail is low (but it may happen!). + * @note the use of inline storage turns swapping of data + * into an expensive operation, involving a temporary. + * This rules out the copy-and-swap idiom. + */ + GenNode& + operator= (GenNode const& o) + { + if (&o != this) + { + data = o.data; + idi = o.idi; + } + return *this; + } + + GenNode& + operator= (GenNode&& o) + { + ASSERT (&o != this); + data = std::forward(o.data); + idi = std::forward(o.idi); + return *this; + } + + //note: NOT defining a swap operation, because swapping inline storage is pointless! + diff --git a/src/lib/variant.hpp b/src/lib/variant.hpp index 66d990384..ad34f0f22 100644 --- a/src/lib/variant.hpp +++ b/src/lib/variant.hpp @@ -452,6 +452,8 @@ namespace lib { return *this; } + //note: NOT defining a swap operation, because swapping inline storage is pointless! + /** diagnostic helper */ operator string() const; diff --git a/tests/gui/interact/state-map-grouping-storage-test.cpp b/tests/gui/interact/state-map-grouping-storage-test.cpp index 575c286e7..a761f23ce 100644 --- a/tests/gui/interact/state-map-grouping-storage-test.cpp +++ b/tests/gui/interact/state-map-grouping-storage-test.cpp @@ -22,7 +22,7 @@ #include "lib/test/run.hpp" -//#include "lib/test/test-helper.hpp" +#include "lib/test/test-helper.hpp" //#include "gui/ctrl/bus-term.hpp" //#include "gui/interact/presentation-state-manager.hpp" #include "gui/interact/state-map-grouping-storage.hpp" @@ -60,7 +60,7 @@ namespace interact{ namespace test { // using proc::control::LUMIERA_ERROR_UNBOUND_ARGUMENTS; -// using lumiera::error::LUMIERA_ERROR_WRONG_TYPE; + using lumiera::error::LUMIERA_ERROR_WRONG_TYPE; namespace { // test fixture... @@ -90,9 +90,10 @@ namespace test { EntryID quack ("quack"); GenNode poodle{"poodle", "Pudel"}; + GenNode toyPoodle{"poodle", "Zwergpudel"}; GenNode pseudoPoodle {"poodle", false}; - GenNode mastiff{"mastiff", "Dogge"}; + GenNode duck{"duck", "Ente"}; StateMapGroupingStorage storage; @@ -103,10 +104,36 @@ namespace test { CHECK (not isnil(storage)); CHECK (1 == storage.size()); - cout << poodle <