Observed issue
==============
Given the type S, some `bt2::internal::SharedObj` class template
instantiation:
* Constructing an S object A from another S object doesn't increment the
internal reference count of the libbabeltrace2 object which A wraps.
* Assigning an S object A from an S object B:
* Doesn't increment the internal reference count of the libbabeltrace2
object which B wraps.
* Doesn't decrement the internal reference count of the libbabeltrace2
object which A wraps.
* Constructing an S object from another moved S object B doesn't
reset B.
* Assigning an S object A from a moved S object B:
* Doesn't reset B.
* Doesn't decrement the internal reference count of the libbabeltrace2
object which A wraps.
Cause
=====
What I thought were copy/move constructor and assignment operators in
`bt2::internal::SharedObj` are in fact simple constructor and assignment
operator templates. In other words, they don't qualify as copy/move
constructor and assignment operators, so the compiler uses default ones.
From [1]:
> A copy constructor of class `T` is a non-template constructor whose
> first parameter is `T&`, `const T&`, `volatile T&`, or `const
> volatile T&`, and either there are no other parameters, or the rest
> of the parameters all have default values.
The same condition (non-template) holds for a move constructor, a copy
assignment operator, and a move assignment operator.
Solution
========
Add copy/move constructors and assignment operators to
`bt2::internal::SharedObj`.
Although you may call an operator=() method template like this:
this->operator=<ObjT, LibObjT>(other)
you can't specify the template parameters when you call a constructor
template: the types of all its parameters need to be deduced.
This is why, to avoid redundant code, I added two common "copy" and
"move" (not real copy/move constructors) private constructor templates
to which the true copy/move constructors and the generic "copy"/"move"
constructors delegate. Those common ones accept a second, unused `int`
parameter to differentiate them from the public, generic "copy"/"move"
constructors.
Known drawbacks
===============
None.
References
==========
[1]: https://en.cppreference.com/w/cpp/language/copy_constructor
Change-Id: I6d8cb7c16da0a79296a482f8d130ab40468cc1a5
Reviewed-on: https://review.lttng.org/c/babeltrace/+/8041
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/10798
Tested-by: jenkins <jenkins@lttng.org>
CI-Build: Philippe Proulx <eeppeliteloop@gmail.com>
* `SharedObj<Something, bt_something, ...>` instance to get
* assigned an instance of
* `SharedObj<SpecificSomething, bt_something, ...>` (copy/move
* `SharedObj<Something, bt_something, ...>` instance to get
* assigned an instance of
* `SharedObj<SpecificSomething, bt_something, ...>` (copy/move
- * constructor and assignment operator), given that
+ * constructors and assignment operators), given that
* `SpecificSomething` inherits `Something`.
*/
template <typename AnyObjT, typename AnyLibObjT, typename AnyRefFuncsT>
* `SpecificSomething` inherits `Something`.
*/
template <typename AnyObjT, typename AnyLibObjT, typename AnyRefFuncsT>
+ /*
+ * Common generic "copy" constructor.
+ *
+ * This constructor is meant to be delegated to by the copy
+ * constructor and the generic "copy" constructor.
+ *
+ * The second parameter, of type `int`, makes it possible to
+ * delegate by deduction as you can't explicit the template
+ * parameters when delegating to a constructor template.
+ */
+ template <typename OtherObjT, typename OtherLibObjT>
+ SharedObj(const SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>& other, int) noexcept :
+ _mObj {other._mObj}
+ {
+ this->_getRef();
+ }
+
+ /*
+ * Common generic "move" constructor.
+ *
+ * See the comment of the common generic "copy" constructor above.
+ */
+ template <typename OtherObjT, typename OtherLibObjT>
+ SharedObj(SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>&& other, int) noexcept :
+ _mObj {other._mObj}
+ {
+ /* Reset moved-from object */
+ other._reset();
+ }
+
public:
/*
* Builds a shared object from `obj` without getting a reference.
public:
/*
* Builds a shared object from `obj` without getting a reference.
- * Generic copy constructor.
+ * Copy constructor.
+ */
+ SharedObj(const SharedObj& other) noexcept : SharedObj {other, 0}
+ {
+ }
+
+ /*
+ * Move constructor.
+ */
+ SharedObj(SharedObj&& other) noexcept : SharedObj {std::move(other), 0}
+ {
+ }
+
+ /*
+ * Copy assignment operator.
+ */
+ SharedObj& operator=(const SharedObj& other) noexcept
+ {
+ /* Use generic "copy" assignment operator */
+ return this->operator=<ObjT, LibObjT>(other);
+ }
+
+ /*
+ * Move assignment operator.
+ */
+ SharedObj& operator=(SharedObj&& other) noexcept
+ {
+ /* Use generic "move" assignment operator */
+ return this->operator=<ObjT, LibObjT>(std::move(other));
+ }
+
+ /*
+ * Generic "copy" constructor.
*
* See the `friend class SharedObj` comment above.
*/
template <typename OtherObjT, typename OtherLibObjT>
SharedObj(const SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>& other) noexcept :
*
* See the `friend class SharedObj` comment above.
*/
template <typename OtherObjT, typename OtherLibObjT>
SharedObj(const SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>& other) noexcept :
- * Generic move constructor.
+ * Generic "move" constructor.
*
* See the `friend class SharedObj` comment above.
*/
template <typename OtherObjT, typename OtherLibObjT>
*
* See the `friend class SharedObj` comment above.
*/
template <typename OtherObjT, typename OtherLibObjT>
- SharedObj(SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>&& other) noexcept : _mObj {other._mObj}
+ SharedObj(SharedObj<OtherObjT, OtherLibObjT, RefFuncsT>&& other) noexcept :
+ SharedObj {std::move(other), 0}
- /* Reset moved-from object */
- other._reset();
- * Generic copy assignment operator.
+ * Generic "copy" assignment operator.
*
* See the `friend class SharedObj` comment above.
*/
*
* See the `friend class SharedObj` comment above.
*/
- * Generic move assignment operator.
+ * Generic "move" assignment operator.
*
* See the `friend class SharedObj` comment above.
*/
*
* See the `friend class SharedObj` comment above.
*/