[PATCH] Add move constructor and move assignment operator for, FileName class

[Jean-Marc Lasgouttes]

This is my take on this patch from yuriy, after 854c9de8 had to be 
reverted. See discussion here:
https://marc.info/?t=161018438900002&r=2&w=2
In the case of move constructor, instead of setting the Private
pointer of the moved FileName to nullptr, set it to a pointer to a
static FileName::Private object. This is needed because the rvalue 
should still be a valid object after a move operation.

Note that this is a shot in the dark : it is not sure why on macOS
some work is done on FileName objects that have a null Private pointer.

This move constructor is supposedly faster, and it pleases Coverity.

Stephan, could you please test it on macOS? This was the platform that 
boke the previous version and lead to a reversion.

JMarcFrom b5876de17aa9cbd56920fdae24f6d316e34682b9 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko <yuriy.ska...@gmail.com>
Date: Thu, 7 Jan 2021 02:27:31 +0200
Subject: [PATCH] Add move constructor and move assignment operator for
 FileName class

This is take 2, after 854c9de8 had to be reverted. See discussion here:
https://marc.info/?t=161018438900002&r=2&w=2

In the case of move constructor, instead of setting the Private
pointer of the moved FileName to nullptr, set it to a pointer to a
static empty_priv object. This is needed because the rvalue should
still be a valid object afterwards.

Note that this is a shot in the dark : it is not sure why on macOS
some work is done on FileName objects that have a null Private pointer.
---
 src/support/FileName.cpp | 27 +++++++++++++++++++++++++--
 src/support/FileName.h   | 17 +++++++++++++----
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/src/support/FileName.cpp b/src/support/FileName.cpp
index 864f38770c..891d45c009 100644
--- a/src/support/FileName.cpp
+++ b/src/support/FileName.cpp
@@ -91,6 +91,12 @@ struct FileName::Private
 		fi.setCaching(fi.exists());
 	}
 	///
+	static Private * empty_d()
+	{
+		static Private empty;
+		return &empty;
+	}
+	///
 	inline void refresh()
 	{
 		fi.refresh();
@@ -124,13 +130,13 @@ struct FileName::Private
 	QFileInfo fi;
 };
 
+
 /////////////////////////////////////////////////////////////////////
 //
 // FileName
 //
 /////////////////////////////////////////////////////////////////////
 
-
 FileName::FileName() : d(new Private)
 {
 }
@@ -146,7 +152,8 @@ FileName::FileName(string const & abs_filename)
 
 FileName::~FileName()
 {
-	delete d;
+	if (d != Private::empty_d())
+		delete d;
 }
 
 
@@ -157,6 +164,13 @@ FileName::FileName(FileName const & rhs) : d(new Private)
 }
 
 
+FileName::FileName(FileName && rhs) noexcept
+	: d(rhs.d)
+{
+	rhs.d = Private::empty_d();
+}
+
+
 FileName::FileName(FileName const & rhs, string const & suffix) : d(new Private)
 {
 	set(rhs, suffix);
@@ -173,6 +187,15 @@ FileName & FileName::operator=(FileName const & rhs)
 }
 
 
+FileName & FileName::operator=(FileName && rhs) noexcept
+{
+	auto temp = rhs.d;
+	rhs.d = d;
+	d = temp;
+	return *this;
+}
+
+
 bool FileName::empty() const
 {
 	return d->name.empty();
diff --git a/src/support/FileName.h b/src/support/FileName.h
index 35f182e428..f86566e4e2 100644
--- a/src/support/FileName.h
+++ b/src/support/FileName.h
@@ -42,15 +42,21 @@ public:
 	 */
 	explicit FileName(std::string const & abs_filename);
 
-	/// copy constructor.
+	/// copy constructor
 	FileName(FileName const &);
 
-	/// constructor with base name and suffix.
+	/// move constructor
+	FileName(FileName &&) noexcept;
+
+	/// constructor with base name and suffix
 	FileName(FileName const & fn, std::string const & suffix);
 
-	///
+	/// copy assign
 	FileName & operator=(FileName const &);
 
+	/// move assign
+	FileName & operator=(FileName &&) noexcept;
+
 	virtual ~FileName();
 	/** Set a new filename.
 	 * \param filename the file in question. Must have an absolute path.
@@ -222,7 +228,10 @@ private:
 	bool copyTo(FileName const &, bool, FileNameSet &) const;
 	///
 	struct Private;
-	Private * const d;
+	// used by move constructor
+	Private * empty_d();
+	// private implmentation
+	Private * d;
 };
 
 
-- 
2.43.0

-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel