Fix: bt2::internal::SharedObj: add real copy/move ctor./operator=()

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>

diff --git a/src/cpp-common/bt2/internal/shared-obj.hpp b/src/cpp-common/bt2/internal/shared-obj.hpp
index d16eb94010e54ebd93f8f0f77d3efcf6877a0702..da8e42f7fb54a15a452ad54d3ba2c71f40532ae2 100644 (file)
--- a/src/cpp-common/bt2/internal/shared-obj.hpp
+++ b/src/cpp-common/bt2/internal/shared-obj.hpp
@@ -39,7 +39,7 @@ class SharedObj final
      * `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>
@@ -53,6 +53,36 @@ private:
     {
     }
 
+    /*
+     * 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.
@@ -93,31 +123,61 @@ public:
     }
 
     /*
-     * 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 :
-        _mObj {other._mObj}
+        SharedObj {other, 0}
     {
-        this->_getRef();
     }
 
     /*
-     * Generic move constructor.
+     * Generic "move" constructor.
      *
      * 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.
      */
@@ -135,7 +195,7 @@ public:
     }
 
     /*
-     * Generic move assignment operator.
+     * Generic "move" assignment operator.
      *
      * See the `friend class SharedObj` comment above.
      */