[libcamera-devel,4/4] Documentation: coding-style: Discourage move on shared_ptr<>

Message ID 20190212222021.28517-5-jacopo@jmondi.org
State Accepted
Headers show
Series
  • libcamera: mixed updated
Related show

Commit Message

Jacopo Mondi Feb. 12, 2019, 10:20 p.m. UTC
Using std::move() on return statement of a method or on the its returned
value prevents the compiler from implementing copy-elision. Discourage
that in the coding style document.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 Documentation/coding-style.rst | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Laurent Pinchart Feb. 12, 2019, 10:41 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Feb 12, 2019 at 11:20:21PM +0100, Jacopo Mondi wrote:
> Using std::move() on return statement of a method or on the its returned
> value prevents the compiler from implementing copy-elision. Discourage
> that in the coding style document.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I haven't explicitly given my SoB for this one, so please don't add it
yourself. To record the advices I gave you, you could use Suggested-by.

This being said, I hereby give you my

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

:-)

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/coding-style.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index 51afef27e9c1..065fbe0ab07b 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -151,6 +151,10 @@ reference for the duration of the operation that borrows it.
>       never by reference. The caller can decide whether to transfer its ownership
>       of the std::shared_ptr<> with std::move() or retain it. The callee shall
>       use std::move() if it needs to store the shared pointer.
> +   * Do not over-use std::move(), as it may prevent copy-elision. In particular
> +     a function returning a std::shared_ptr<> value shall not use std::move() in
> +     its return statements, and its callers shall not wrap the function call
> +     with std::move().
>     * Borrowed references to shared objects are passed as references to the
>       objects themselves, not to the std::shared_ptr<>, with the same rules as
>       for single owner objects.
Niklas Söderlund Feb. 13, 2019, 11:02 a.m. UTC | #2
Hi Jacopo,

Thanks for your work.

On 2019-02-12 23:20:21 +0100, Jacopo Mondi wrote:
> Using std::move() on return statement of a method or on the its returned
> value prevents the compiler from implementing copy-elision. Discourage
> that in the coding style document.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  Documentation/coding-style.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index 51afef27e9c1..065fbe0ab07b 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -151,6 +151,10 @@ reference for the duration of the operation that borrows it.
>       never by reference. The caller can decide whether to transfer its ownership
>       of the std::shared_ptr<> with std::move() or retain it. The callee shall
>       use std::move() if it needs to store the shared pointer.
> +   * Do not over-use std::move(), as it may prevent copy-elision. In particular
> +     a function returning a std::shared_ptr<> value shall not use std::move() in
> +     its return statements, and its callers shall not wrap the function call
> +     with std::move().
>     * Borrowed references to shared objects are passed as references to the
>       objects themselves, not to the std::shared_ptr<>, with the same rules as
>       for single owner objects.
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
index 51afef27e9c1..065fbe0ab07b 100644
--- a/Documentation/coding-style.rst
+++ b/Documentation/coding-style.rst
@@ -151,6 +151,10 @@  reference for the duration of the operation that borrows it.
      never by reference. The caller can decide whether to transfer its ownership
      of the std::shared_ptr<> with std::move() or retain it. The callee shall
      use std::move() if it needs to store the shared pointer.
+   * Do not over-use std::move(), as it may prevent copy-elision. In particular
+     a function returning a std::shared_ptr<> value shall not use std::move() in
+     its return statements, and its callers shall not wrap the function call
+     with std::move().
    * Borrowed references to shared objects are passed as references to the
      objects themselves, not to the std::shared_ptr<>, with the same rules as
      for single owner objects.