Remove NonCopyable

The class NonCopyable is, like the name says, supposed to disallow
copying. But should it allow moving?

For a long time, NonCopyable used to not allow moving. (It declared
a deleted copy constructor and assigment operator without declaring
a move constructor and assignment operator, making the compiler
implicitly delete the move constructor and assignment operator.)
That's fine if the classes that inherit from NonCopyable don't need
to be movable or if writing the move constructor and assignment
operator by hand is fine, but that's not the case for all classes,
as I discovered when I was working on the DirectoryBlob PR.

Because of that, I decided to make NonCopyable movable in c7602cc,
allowing me to use NonCopyable in DirectoryBlob.h. That was however
an unfortunate decision, because some of the classes that inherit
from NonCopyable have incorrect behavior when moved by default-
generated move constructors and assignment operators, and do not
explicitly delete the move constructors and assignment operators,
relying on NonCopyable being non-movable.

So what can we do about this? There are four solutions that I can
think of:

1. Make NonCopyable non-movable and tell DirectoryBlob to suck it.

2. Keep allowing moving NonCopyable, and expect that classes that
   don't support moving will delete the move constructor and
   assignment operator manually. Not only is this inconsistent
   (having classes disallow copying one way and disallow moving
   another way), but deleting the move constructor and assignment
   operator manually is too easy to forget compared to how tricky
   the resulting problems are.

3. Have one "MovableNonCopyable" and one "NonMovableNonCopyable".
   It works, but it feels rather silly...

4. Don't have a NonCopyable class at all. Considering that deleting
   the copy constructor and assignment operator only takes two lines
   of code, I don't see much of a reason to keep NonCopyable. I
   suppose that there was more of a point in having NonCopyable back
   in the pre-C++11 days, when it wasn't possible to use "= delete".

I decided to go with the fourth one (like the commit title says).
The implementation of the commit is fairly straight-forward, though
I would like to point out that I skipped adding "= delete" lines
for classes whose only reason for being uncopyable is that they
contain uncopyable classes like File::IOFile and std::unique_ptr,
because the compiler makes such classes uncopyable automatically.
This commit is contained in:
JosJuice 2017-08-04 23:57:12 +02:00
parent 6e6864fcbf
commit 09f3f9b41a
19 changed files with 75 additions and 54 deletions

View File

@ -19,14 +19,18 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/File.h" #include "Common/File.h"
#include "Common/NonCopyable.h"
class WaveFileWriter : NonCopyable class WaveFileWriter
{ {
public: public:
WaveFileWriter(); WaveFileWriter();
~WaveFileWriter(); ~WaveFileWriter();
WaveFileWriter(const WaveFileWriter&) = delete;
WaveFileWriter& operator=(const WaveFileWriter&) = delete;
WaveFileWriter(WaveFileWriter&&) = delete;
WaveFileWriter& operator=(WaveFileWriter&&) = delete;
bool Start(const std::string& filename, unsigned int HLESampleRate); bool Start(const std::string& filename, unsigned int HLESampleRate);
void Stop(); void Stop();

View File

@ -10,7 +10,6 @@
#include "Common/Assert.h" #include "Common/Assert.h"
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/MemoryUtil.h" #include "Common/MemoryUtil.h"
#include "Common/NonCopyable.h"
// Everything that needs to generate code should inherit from this. // Everything that needs to generate code should inherit from this.
// You get memory management for free, plus, you can use all emitter functions without // You get memory management for free, plus, you can use all emitter functions without
@ -18,7 +17,7 @@
// Example implementation: // Example implementation:
// class JIT : public CodeBlock<ARMXEmitter> {} // class JIT : public CodeBlock<ARMXEmitter> {}
template <class T> template <class T>
class CodeBlock : public T, NonCopyable class CodeBlock : public T
{ {
private: private:
// A privately used function to set the executable RAM space to something invalid. // A privately used function to set the executable RAM space to something invalid.
@ -37,11 +36,16 @@ protected:
std::vector<CodeBlock*> m_children; std::vector<CodeBlock*> m_children;
public: public:
CodeBlock() = default;
virtual ~CodeBlock() virtual ~CodeBlock()
{ {
if (region) if (region)
FreeCodeSpace(); FreeCodeSpace();
} }
CodeBlock(const CodeBlock&) = delete;
CodeBlock& operator=(const CodeBlock&) = delete;
CodeBlock(CodeBlock&&) = delete;
CodeBlock& operator=(CodeBlock&&) = delete;
// Call this before you generate any code. // Call this before you generate any code.
void AllocCodeSpace(size_t size) void AllocCodeSpace(size_t size)

View File

@ -136,7 +136,6 @@
<ClInclude Include="MsgHandler.h" /> <ClInclude Include="MsgHandler.h" />
<ClInclude Include="NandPaths.h" /> <ClInclude Include="NandPaths.h" />
<ClInclude Include="Network.h" /> <ClInclude Include="Network.h" />
<ClInclude Include="NonCopyable.h" />
<ClInclude Include="PcapFile.h" /> <ClInclude Include="PcapFile.h" />
<ClInclude Include="Profiler.h" /> <ClInclude Include="Profiler.h" />
<ClInclude Include="ScopeGuard.h" /> <ClInclude Include="ScopeGuard.h" />

View File

@ -237,7 +237,6 @@
<Filter>GL\GLInterface</Filter> <Filter>GL\GLInterface</Filter>
</ClInclude> </ClInclude>
<ClInclude Include="Assert.h" /> <ClInclude Include="Assert.h" />
<ClInclude Include="NonCopyable.h" />
<ClInclude Include="Analytics.h" /> <ClInclude Include="Analytics.h" />
<ClInclude Include="Semaphore.h" /> <ClInclude Include="Semaphore.h" />
<ClInclude Include="MD5.h" /> <ClInclude Include="MD5.h" />

View File

@ -9,14 +9,13 @@
#include <string> #include <string>
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/NonCopyable.h"
namespace File namespace File
{ {
// simple wrapper for cstdlib file functions to // simple wrapper for cstdlib file functions to
// hopefully will make error checking easier // hopefully will make error checking easier
// and make forgetting an fclose() harder // and make forgetting an fclose() harder
class IOFile : public NonCopyable class IOFile
{ {
public: public:
IOFile(); IOFile();
@ -25,6 +24,9 @@ public:
~IOFile(); ~IOFile();
IOFile(const IOFile&) = delete;
IOFile& operator=(const IOFile&) = delete;
IOFile(IOFile&& other) noexcept; IOFile(IOFile&& other) noexcept;
IOFile& operator=(IOFile&& other) noexcept; IOFile& operator=(IOFile&& other) noexcept;

View File

@ -12,7 +12,6 @@
#include <sys/stat.h> #include <sys/stat.h>
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/NonCopyable.h"
#ifdef _WIN32 #ifdef _WIN32
#include "Common/StringUtil.h" #include "Common/StringUtil.h"

View File

@ -9,7 +9,6 @@
#include "Common/BitSet.h" #include "Common/BitSet.h"
#include "Common/Logging/Log.h" #include "Common/Logging/Log.h"
#include "Common/NonCopyable.h"
// pure virtual interface // pure virtual interface
class LogListener class LogListener
@ -28,7 +27,7 @@ public:
}; };
}; };
class LogManager : NonCopyable class LogManager
{ {
public: public:
static LogManager* GetInstance(); static LogManager* GetInstance();
@ -66,6 +65,11 @@ private:
LogManager(); LogManager();
~LogManager(); ~LogManager();
LogManager(const LogManager&) = delete;
LogManager& operator=(const LogManager&) = delete;
LogManager(LogManager&&) = delete;
LogManager& operator=(LogManager&&) = delete;
LogTypes::LOG_LEVELS m_level; LogTypes::LOG_LEVELS m_level;
std::array<LogContainer, LogTypes::NUMBER_OF_LOGS> m_log{}; std::array<LogContainer, LogTypes::NUMBER_OF_LOGS> m_log{};
std::array<LogListener*, LogListener::NUMBER_OF_LISTENERS> m_listeners{}; std::array<LogListener*, LogListener::NUMBER_OF_LISTENERS> m_listeners{};

View File

@ -1,19 +0,0 @@
// Copyright 2015 Dolphin Emulator Project
// Licensed under GPLv2+
// Refer to the license.txt file included.
#pragma once
// An inheritable class to disallow the copy constructor and operator= functions
class NonCopyable
{
protected:
constexpr NonCopyable() = default;
~NonCopyable() = default;
NonCopyable(const NonCopyable&) = delete;
NonCopyable& operator=(const NonCopyable&) = delete;
NonCopyable(NonCopyable&&) = default;
NonCopyable& operator=(NonCopyable&&) = default;
};

View File

@ -18,9 +18,8 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/File.h" #include "Common/File.h"
#include "Common/NonCopyable.h"
class PCAP final : public NonCopyable class PCAP final
{ {
public: public:
// Takes ownership of the file object. Assumes the file object is already // Takes ownership of the file object. Assumes the file object is already

View File

@ -12,7 +12,6 @@
#include <vector> #include <vector>
#include "Common/IniFile.h" #include "Common/IniFile.h"
#include "Common/NonCopyable.h"
#include "Core/HW/EXI/EXI_Device.h" #include "Core/HW/EXI/EXI_Device.h"
#include "Core/HW/SI/SI_Device.h" #include "Core/HW/SI/SI_Device.h"
#include "Core/TitleDatabase.h" #include "Core/TitleDatabase.h"
@ -52,7 +51,7 @@ enum GPUDeterminismMode
struct BootParameters; struct BootParameters;
struct SConfig : NonCopyable struct SConfig
{ {
// Wii Devices // Wii Devices
bool m_WiiSDCard; bool m_WiiSDCard;
@ -318,6 +317,11 @@ struct SConfig : NonCopyable
bool m_SSLDumpRootCA; bool m_SSLDumpRootCA;
bool m_SSLDumpPeerCert; bool m_SSLDumpPeerCert;
SConfig(const SConfig&) = delete;
SConfig& operator=(const SConfig&) = delete;
SConfig(SConfig&&) = delete;
SConfig& operator=(SConfig&&) = delete;
// Save settings // Save settings
void SaveSettings(); void SaveSettings();

View File

@ -8,7 +8,6 @@
#include <string> #include <string>
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/NonCopyable.h"
class PCAP; class PCAP;
@ -51,7 +50,7 @@ public:
// A capture logger implementation that logs to PCAP files in a custom // A capture logger implementation that logs to PCAP files in a custom
// packet-based format. // packet-based format.
class PCAPDSPCaptureLogger final : public DSPCaptureLogger, NonCopyable class PCAPDSPCaptureLogger final : public DSPCaptureLogger
{ {
public: public:
// Automatically creates a writeable file (truncate existing file). // Automatically creates a writeable file (truncate existing file).

View File

@ -9,7 +9,6 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/NandPaths.h" #include "Common/NandPaths.h"
#include "Common/NonCopyable.h"
#include "Common/Swap.h" #include "Common/Swap.h"
#include "Common/Timer.h" #include "Common/Timer.h"
@ -295,7 +294,7 @@ public:
std::string m_filename; std::string m_filename;
}; };
class GCMemcard : NonCopyable class GCMemcard
{ {
private: private:
bool m_valid; bool m_valid;
@ -317,6 +316,12 @@ private:
public: public:
explicit GCMemcard(const std::string& fileName, bool forceCreation = false, explicit GCMemcard(const std::string& fileName, bool forceCreation = false,
bool shift_jis = false); bool shift_jis = false);
GCMemcard(const GCMemcard&) = delete;
GCMemcard& operator=(const GCMemcard&) = delete;
GCMemcard(GCMemcard&&) = default;
GCMemcard& operator=(GCMemcard&&) = default;
bool IsValid() const { return m_valid; } bool IsValid() const { return m_valid; }
bool IsShiftJIS() const; bool IsShiftJIS() const;
bool Save(); bool Save();

View File

@ -10,19 +10,24 @@
#include <vector> #include <vector>
#include "Common/Event.h" #include "Common/Event.h"
#include "Common/NonCopyable.h"
#include "Core/HW/GCMemcard/GCMemcard.h" #include "Core/HW/GCMemcard/GCMemcard.h"
// Uncomment this to write the system data of the memorycard from directory to disc // Uncomment this to write the system data of the memorycard from directory to disc
//#define _WRITE_MC_HEADER 1 //#define _WRITE_MC_HEADER 1
void MigrateFromMemcardFile(const std::string& directory_name, int card_index); void MigrateFromMemcardFile(const std::string& directory_name, int card_index);
class GCMemcardDirectory : public MemoryCardBase, NonCopyable class GCMemcardDirectory : public MemoryCardBase
{ {
public: public:
GCMemcardDirectory(const std::string& directory, int slot, u16 size_mbits, bool shift_jis, GCMemcardDirectory(const std::string& directory, int slot, u16 size_mbits, bool shift_jis,
int game_id); int game_id);
~GCMemcardDirectory(); ~GCMemcardDirectory();
GCMemcardDirectory(const GCMemcardDirectory&) = delete;
GCMemcardDirectory& operator=(const GCMemcardDirectory&) = delete;
GCMemcardDirectory(GCMemcardDirectory&&) = default;
GCMemcardDirectory& operator=(GCMemcardDirectory&&) = default;
void FlushToFile(); void FlushToFile();
void FlushThread(); void FlushThread();
s32 Read(u32 src_address, s32 length, u8* dest_address) override; s32 Read(u32 src_address, s32 length, u8* dest_address) override;

View File

@ -8,7 +8,6 @@
#include <memory> #include <memory>
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/NonCopyable.h"
// All the templated and very repetitive MMIO-related code is isolated in this // All the templated and very repetitive MMIO-related code is isolated in this
// file for easier reading. It mostly contains code related to handling methods // file for easier reading. It mostly contains code related to handling methods
@ -119,7 +118,7 @@ public:
// inlinable, we need to provide some of the implementation of these two // inlinable, we need to provide some of the implementation of these two
// classes here and can't just use a forward declaration. // classes here and can't just use a forward declaration.
template <typename T> template <typename T>
class ReadHandler : public NonCopyable class ReadHandler
{ {
public: public:
ReadHandler(); ReadHandler();
@ -155,7 +154,7 @@ private:
std::function<T(u32)> m_ReadFunc; std::function<T(u32)> m_ReadFunc;
}; };
template <typename T> template <typename T>
class WriteHandler : public NonCopyable class WriteHandler
{ {
public: public:
WriteHandler(); WriteHandler();

View File

@ -14,7 +14,6 @@
#include "Common/Event.h" #include "Common/Event.h"
#include "Common/FifoQueue.h" #include "Common/FifoQueue.h"
#include "Common/Flag.h" #include "Common/Flag.h"
#include "Common/NonCopyable.h"
#include "Core/HW/Wiimote.h" #include "Core/HW/Wiimote.h"
#include "Core/HW/WiimoteCommon/WiimoteConstants.h" #include "Core/HW/WiimoteCommon/WiimoteConstants.h"
#include "Core/HW/WiimoteCommon/WiimoteHid.h" #include "Core/HW/WiimoteCommon/WiimoteHid.h"
@ -24,9 +23,14 @@ class PointerWrap;
namespace WiimoteReal namespace WiimoteReal
{ {
class Wiimote : NonCopyable class Wiimote
{ {
public: public:
Wiimote(const Wiimote&) = delete;
Wiimote& operator=(const Wiimote&) = delete;
Wiimote(Wiimote&&) = default;
Wiimote& operator=(Wiimote&&) = default;
virtual ~Wiimote() {} virtual ~Wiimote() {}
// This needs to be called in derived destructors! // This needs to be called in derived destructors!
void Shutdown(); void Shutdown();

View File

@ -50,7 +50,6 @@ typedef struct pollfd pollfd_t;
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/Logging/Log.h" #include "Common/Logging/Log.h"
#include "Common/NonCopyable.h"
#include "Core/HW/Memmap.h" #include "Core/HW/Memmap.h"
#include "Core/IOS/IOS.h" #include "Core/IOS/IOS.h"
#include "Core/IOS/Network/IP/Top.h" #include "Core/IOS/Network/IP/Top.h"
@ -207,7 +206,7 @@ public:
void operator=(WiiSocket const&) = delete; void operator=(WiiSocket const&) = delete;
}; };
class WiiSockMan : public ::NonCopyable class WiiSockMan
{ {
public: public:
static s32 GetNetErrorCode(s32 ret, const char* caller, bool isRW); static s32 GetNetErrorCode(s32 ret, const char* caller, bool isRW);
@ -249,6 +248,10 @@ public:
private: private:
WiiSockMan() = default; WiiSockMan() = default;
WiiSockMan(const WiiSockMan&) = delete;
WiiSockMan& operator=(const WiiSockMan&) = delete;
WiiSockMan(WiiSockMan&&) = delete;
WiiSockMan& operator=(WiiSockMan&&) = delete;
std::unordered_map<s32, WiiSocket> WiiSockets; std::unordered_map<s32, WiiSocket> WiiSockets;
s32 errno_last; s32 errno_last;

View File

@ -9,8 +9,6 @@
#include <QObject> #include <QObject>
#include <QVector> #include <QVector>
#include "Common/NonCopyable.h"
#include "Core/NetPlayClient.h" #include "Core/NetPlayClient.h"
#include "Core/NetPlayServer.h" #include "Core/NetPlayServer.h"
@ -23,11 +21,16 @@ class GameListModel;
class InputConfig; class InputConfig;
// UI settings to be stored in the config directory. // UI settings to be stored in the config directory.
class Settings final : public QObject, NonCopyable class Settings final : public QObject
{ {
Q_OBJECT Q_OBJECT
public: public:
Settings(const Settings&) = delete;
Settings& operator=(const Settings&) = delete;
Settings(Settings&&) = delete;
Settings& operator=(Settings&&) = delete;
static Settings& Instance(); static Settings& Instance();
// UI // UI

View File

@ -9,17 +9,21 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/GL/GLUtil.h" #include "Common/GL/GLUtil.h"
#include "Common/NonCopyable.h"
#include "VideoBackends/OGL/Render.h" #include "VideoBackends/OGL/Render.h"
namespace OGL namespace OGL
{ {
class SamplerCache : NonCopyable class SamplerCache
{ {
public: public:
SamplerCache(); SamplerCache();
~SamplerCache(); ~SamplerCache();
SamplerCache(const SamplerCache&) = delete;
SamplerCache& operator=(const SamplerCache&) = delete;
SamplerCache(SamplerCache&&) = delete;
SamplerCache& operator=(SamplerCache&&) = delete;
void SetSamplerState(int stage, const TexMode0& tm0, const TexMode1& tm1, bool custom_tex); void SetSamplerState(int stage, const TexMode0& tm0, const TexMode1& tm1, bool custom_tex);
void Clear(); void Clear();
void BindNearestSampler(int stage); void BindNearestSampler(int stage);

View File

@ -9,7 +9,6 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/Hash.h" #include "Common/Hash.h"
#include "Common/NonCopyable.h"
// m_components // m_components
enum enum
@ -102,10 +101,15 @@ struct hash<PortableVertexDeclaration>
// Note that this class can't just invent arbitrary vertex formats out of its input - // Note that this class can't just invent arbitrary vertex formats out of its input -
// all the data loading code must always be made compatible. // all the data loading code must always be made compatible.
class NativeVertexFormat : NonCopyable class NativeVertexFormat
{ {
public: public:
virtual ~NativeVertexFormat() {} virtual ~NativeVertexFormat() {}
NativeVertexFormat(const NativeVertexFormat&) = delete;
NativeVertexFormat& operator=(const NativeVertexFormat&) = delete;
NativeVertexFormat(NativeVertexFormat&&) = default;
NativeVertexFormat& operator=(NativeVertexFormat&&) = default;
u32 GetVertexStride() const { return vtx_decl.stride; } u32 GetVertexStride() const { return vtx_decl.stride; }
const PortableVertexDeclaration& GetVertexDeclaration() const { return vtx_decl; } const PortableVertexDeclaration& GetVertexDeclaration() const { return vtx_decl; }
protected: protected: