[RFC,v2,1/9] libcamera: base: log: Remove move constructor

22710 diff mbox series

C++17 guarantees move and copy elision in certain cases,
such as when returning a prvalue of the same type as the
return type of the function.

This is what the `_log()` functions do, thus there is no need
for the move constructor, so remove it. Furthermore, do not
just remove the implementation, but instead delete it as well.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
---
 include/libcamera/base/log.h |  4 +---
 src/libcamera/base/log.cpp   | 19 -------------------
 2 files changed, 1 insertion(+), 22 deletions(-)

Hi Barnabás

On Thu, Jan 30, 2025 at 07:58:20PM +0000, Barnabás Pőcze wrote:
> C++17 guarantees move and copy elision in certain cases,
> such as when returning a prvalue of the same type as the
> return type of the function.
>
> This is what the `_log()` functions do, thus there is no need
> for the move constructor, so remove it. Furthermore, do not
> just remove the implementation, but instead delete it as well.
>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

Now that we're past c++11 this is fine

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  include/libcamera/base/log.h |  4 +---
>  src/libcamera/base/log.cpp   | 19 -------------------
>  2 files changed, 1 insertion(+), 22 deletions(-)
>
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index 620930125..b3050eedb 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -61,8 +61,6 @@ public:
>  	LogMessage(const char *fileName, unsigned int line,
>  		   const LogCategory &category, LogSeverity severity,
>  		   const std::string &prefix = std::string());
> -
> -	LogMessage(LogMessage &&);
>  	~LogMessage();
>
>  	std::ostream &stream() { return msgStream_; }
> @@ -75,7 +73,7 @@ public:
>  	const std::string msg() const { return msgStream_.str(); }
>
>  private:
> -	LIBCAMERA_DISABLE_COPY(LogMessage)
> +	LIBCAMERA_DISABLE_COPY_AND_MOVE(LogMessage)
>
>  	void init(const char *fileName, unsigned int line);
>
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 3a656b8f0..1917c3254 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -880,25 +880,6 @@ LogMessage::LogMessage(const char *fileName, unsigned int line,
>  	init(fileName, line);
>  }
>
> -/**
> - * \brief Move-construct a log message
> - * \param[in] other The other message
> - *
> - * The move constructor is meant to support the _log() functions. Thanks to copy
> - * elision it will likely never be called, but C++11 only permits copy elision,
> - * it doesn't enforce it unlike C++17. To avoid potential link errors depending
> - * on the compiler type and version, and optimization level, the move
> - * constructor is defined even if it will likely never be called, and ensures
> - * that the destructor of the \a other message will not output anything to the
> - * log by setting the severity to LogInvalid.
> - */
> -LogMessage::LogMessage(LogMessage &&other)
> -	: msgStream_(std::move(other.msgStream_)), category_(other.category_),
> -	  severity_(other.severity_)
> -{
> -	other.severity_ = LogInvalid;
> -}
> -
>  void LogMessage::init(const char *fileName, unsigned int line)
>  {
>  	/* Log the timestamp, severity and file information. */
> --
> 2.48.1
>
>

Hi Barnabás,

Thank you for the patch.

On Thu, Jan 30, 2025 at 07:58:20PM +0000, Barnabás Pőcze wrote:
> C++17 guarantees move and copy elision in certain cases,
> such as when returning a prvalue of the same type as the
> return type of the function.
> 
> This is what the `_log()` functions do, thus there is no need
> for the move constructor, so remove it. Furthermore, do not
> just remove the implementation, but instead delete it as well.
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

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

> ---
>  include/libcamera/base/log.h |  4 +---
>  src/libcamera/base/log.cpp   | 19 -------------------
>  2 files changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index 620930125..b3050eedb 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -61,8 +61,6 @@ public:
>  	LogMessage(const char *fileName, unsigned int line,
>  		   const LogCategory &category, LogSeverity severity,
>  		   const std::string &prefix = std::string());
> -
> -	LogMessage(LogMessage &&);
>  	~LogMessage();
>  
>  	std::ostream &stream() { return msgStream_; }
> @@ -75,7 +73,7 @@ public:
>  	const std::string msg() const { return msgStream_.str(); }
>  
>  private:
> -	LIBCAMERA_DISABLE_COPY(LogMessage)
> +	LIBCAMERA_DISABLE_COPY_AND_MOVE(LogMessage)
>  
>  	void init(const char *fileName, unsigned int line);
>  
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 3a656b8f0..1917c3254 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -880,25 +880,6 @@ LogMessage::LogMessage(const char *fileName, unsigned int line,
>  	init(fileName, line);
>  }
>  
> -/**
> - * \brief Move-construct a log message
> - * \param[in] other The other message
> - *
> - * The move constructor is meant to support the _log() functions. Thanks to copy
> - * elision it will likely never be called, but C++11 only permits copy elision,
> - * it doesn't enforce it unlike C++17. To avoid potential link errors depending
> - * on the compiler type and version, and optimization level, the move
> - * constructor is defined even if it will likely never be called, and ensures
> - * that the destructor of the \a other message will not output anything to the
> - * log by setting the severity to LogInvalid.
> - */
> -LogMessage::LogMessage(LogMessage &&other)
> -	: msgStream_(std::move(other.msgStream_)), category_(other.category_),
> -	  severity_(other.severity_)
> -{
> -	other.severity_ = LogInvalid;
> -}
> -
>  void LogMessage::init(const char *fileName, unsigned int line)
>  {
>  	/* Log the timestamp, severity and file information. */