Moving from a ValueTree

#1

So, iiuc, copying a ValueTree means that the copy will refer to the same underlying data as the original ValueTree, but it will have no listeners (so the original ValueTree's listeners are not copied). That’s maybe a bit unintuitive at first, but OK – it’s a useful property.

But here’s something really confusing: what are the move semantics of a ValueTree? If I move-from a ValueTree, will the new tree also have no listeners? I can’t figure that out by looking at the JUCE code.

This is crucial because I have a class – let’s call it Widget – that has an internal ValueTree member, and is listening to that. (for example, CachedValue is doing the same thing, I think it’s a reasonably common pattern).

Now I need to have a std::vector of Widgets. Therefore, Widget needs to have a non-throwing move constructor.

How do I correctly implement this for Widget, without producing Widgets that are broken because they don’t listen to their internal ValueTree anymore? (I noticed that CachedValue has an implicitly declared move constructor, isn’t that broken, too?)

Note also that ValueTree::addListener isn’t noexcept, therefore it can’t be used in a move constructor.

5 Likes
#2

Did you ever figure this out? I’m struggling with the same thing.

0 Likes
#3

I am not sure, if that is properly solvable, since when a class adds itself as listener to a ValueTree, and the ValueTree is copied including the listeners, how would the listening class know, it has to deregister from two ValueTrees on destruction?

I think a silent copying of listeners can be just as unintuitive as not copying it.

0 Likes
#4

imho the solution would be to implement a proper move constructor for ValueTree, which moves the tree’s listener list over into the new tree instance. Since this does not require any listeners to be added to or removed from a listener list, it can be made noexcept.

This would leave the moved-from ValueTree without any listeners, which is fine.

1 Like
#5

I’m curious. Why do questions like this get zero attention from the JUCE team? Is it because being able to move from value trees is just something they think you shouldn’t do, or just something they never had to do, and thus didn’t think it would need to be implemented?

@timur can you show what a proper move constructor for ValueTree would look like?

0 Likes
#6

OK, so the current ValueTree move constructor looks like this:

ValueTree::ValueTree (ValueTree&& other) noexcept
    : object (std::move (other.object))
{
    if (object != nullptr)
        object->valueTreesWithListeners.removeValue (&other);
}

This does not provide what I would consider correct move semantics, for the reason explained above: the new ValueTree does not have the Listeners of the moved-from object. What I think this should do is simply:

ValueTree::ValueTree (ValueTree&& other) noexcept
    : object (std::move (other.object)),
      listeners (std::move (other.listeners))
{}

provided that ListenerList has a correct nothrow-move-ctor (which I do not have the time to check right now).

0 Likes
#7

I mean, that’s not 100% clean either, because then moving a ValueTree will give the new tree the listeners of the old, but copying a ValueTree will not give the new tree the listeners of the old. It almost feels like the semantics of copying a ValueTree inherently do something else than copying (reminds me a bit of std::auto_ptr), and that makes_moving_ a ValueTree more odd.

My guess is that the JUCE team simply does not want to mess with the semantics of such a widely-used class :wink:

1 Like
#8

why can’t there be a move assign operator that copies correctly if you do something like this?

ValueTree otherTree ("SourceTree");
ValueTree tree ("CopyTree");
tree = std:move (otherTree);

?

@jules @ed95 @t0m Why does the ValueTree class have

/** Move constructor */
    ValueTree (ValueTree&&) noexcept;

but no Move Assign operator?

you guys didn’t follow the rule of 3/5/0 here…

Rule of five
Because the presence of a user-defined destructor, copy-constructor, or copy-assignment 
operator prevents implicit definition of the move constructor and the move assignment
operator, any class for which move semantics are desirable, has to declare all 
five special member functions:

https://en.cppreference.com/w/cpp/language/rule_of_three

2 Likes
#9

There’s certainly an element of that; we’ll need to spend some time thinking over the best way to do things. No one has commented on this as there’s not much to say - it’s clearly something we could do better, and looking at it has gone on the backlog.

1 Like