Fix: bt2::internal::SharedObj: add real copy/move ctor./operator=()
authorPhilippe Proulx <eeppeliteloop@gmail.com>
Sat, 14 May 2022 16:28:52 +0000 (12:28 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Mon, 11 Sep 2023 15:24:02 +0000 (11:24 -0400)
commite60a53f18299b25741db35815997690f02f9a0f7
treed1c22f0b00c908bf7708395e6f59ea0b5c8f324a
parentc9c0b6e232066c94b2f4637cf913762e8e71e13a
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>
src/cpp-common/bt2/internal/shared-obj.hpp
This page took 0.025636 seconds and 4 git commands to generate.