ichthyo/LUMIERA.clone
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

TreeMutator binding: fix collection binding to support move-only types

Browse source 
unintentionally we used copy construction in the builder expression,
wenn passing in the CollectionBinding to the ChildCollectionMutator.

The problem is that CollectionBinding owns a shaddow buffer, where
the contents of the target collection are moved temporarily while
applying the diff. The standard implementation of copy construction
would cause a copy of that shaddow buffer, which boils down to
a copy of the storage of the target collection.

If we want to support move-only types in the collection, most notably
std::unique_ptr, we can thus only use the move constructor. Beyond that
there is no problem, since we're only ever moving elements, and new
elements will be move constructed via emplace() or emplace_back()
This commit is contained in:
Fischlurch 2016-10-03 20:08:54 +02:00
parent d58f8c853a
commit bada8ecffd
3 changed files with 13 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/lib/diff/tree-mutator-collection-binding.hpp
View file

@ -128,6 +128,12 @@ namespace diff{
, openSub(u)
{ }
// allow move construction only,
// to enable use of unique_ptr in collections
CollectionBinding(CollectionBinding&&) = default;
CollectionBinding(CollectionBinding&) = delete;
/* === content manipulation API === */
@ -259,9 +265,9 @@ namespace diff{
public:
ChildCollectionMutator(BIN wiringClosures, PAR&& chain)
ChildCollectionMutator(BIN&& wiringClosures, PAR&& chain)
: PAR(std::forward<PAR>(chain))
, binding_(wiringClosures)
, binding_(forward<BIN>(wiringClosures))
, pos_(binding_.initMutation())
{ }
@ -280,7 +286,7 @@ namespace diff{
{
if (binding_.isApplicable(n))
{
binding_.inject (binding_.construct(n));
binding_.inject (std::move (binding_.construct(n)));
return true;
}
else

3
src/lib/diff/tree-mutator.hpp
View file

@ -411,7 +411,8 @@ namespace diff{
* - the _matcher closure_ (CollectionBindingBuilder::matchElement) defines
* how to determine, if an implementation data element "matches" a given diff spec
* - the _constructor closure_ (CollectionBindingBuilder::constructFrom) defines how
* to build a new implementation data element from the spec of an `INS` diff verb
* to build a new implementation data element from the spec of an `INS` diff verb.
* Note: the result will be moved (move-constructed) into the target container.
* - the optional _selector closure_ (CollectionBindingBuilder::isApplicableIf)
* allows to limit applicability of this whole binding (layer) to only some
* diff specs. E.g., we may set up a binding for elements with value semantics

4
tests/gui/test/mock-elm.hpp
View file

@ -92,7 +92,7 @@ namespace test{
using std::string;
class MockElm;
using PMockElm = std::shared_ptr<MockElm>;
using PMockElm = std::unique_ptr<MockElm>;
/**
@ -252,7 +252,7 @@ namespace test{
})
.constructFrom ([&](GenNode const& spec) -> PMockElm
{
return std::make_shared<MockElm>(spec.idi, this->uiBus_); // create a child element wired via this Element's BusTerm
return std::make_unique<MockElm>(spec.idi, this->uiBus_); // create a child element wired via this Element's BusTerm
})
.buildChildMutator ([&](PMockElm& target, GenNode::ID const& subID, TreeMutator::Handle buff) -> bool
{