Closed meyerj closed 4 years ago
LGTM
I am not so sure yet about the new assignment semantics of RTT::Property<T>
. I was actually surprised that copy construction and copy assignment had different semantics, but I think it is more consistent like it is now:
RTT::Property<T>
of the same type creates a deep copy (value semantics).RTT::base::PropertyBase*
pointer only copies the name and description, but has reference semantics for the actual value. The type of the source property still has to match or the target property becomes invalid (ready() == false
). The original property and the copy will share the same data source (mirroring).On the other hand some existing code might depend on the old semantics. RTT unit tests still pass successfully.
Probably best we can do. The inconsistencies in application will have to be dealt with ... :-(
RTT::Attribute
also has the same, more consistent semantics:
Attribute( const Attribute<T>& a)
creates a deep copy.Attribute<T>& operator=( const Attribute<T>& a)
creates a deep copy.Attribute( base::AttributeBase* ab)
creates a shallow copy (mirror).Attribute<T>& operator=(base::AttributeBase* ab)
creates a shallow copy (mirror).So I start to think that it was actually a bug.
This pull request eliminates some compiler warnings with newer compilers (GCC 9 in upcoming Ubuntu 20.04, with Boost 1.72). Some warnings are older and already appeared before.
The patch also includes an API update proposal for more consistent copy assignment semantics of
RTT::Property<T>
, which was inconsistent with copy construction, and a slightly modified API ofRTT::PropertyBag
for adding properties, that likely has no effect in practice.