From 6e6864fcbf8af1c1655b63dee44588a77ff10068 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 4 Aug 2017 20:45:25 +0200 Subject: [PATCH 1/2] Revert "DirectoryBlob: Use NonCopyable" This reverts commit a7a8e467b6f0efd9f6236221a346f221edefd059. --- Source/Core/DiscIO/DirectoryBlob.h | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Source/Core/DiscIO/DirectoryBlob.h b/Source/Core/DiscIO/DirectoryBlob.h index cc15db64f9..666694e508 100644 --- a/Source/Core/DiscIO/DirectoryBlob.h +++ b/Source/Core/DiscIO/DirectoryBlob.h @@ -15,7 +15,6 @@ #include "Common/CommonTypes.h" #include "Common/FileUtil.h" -#include "Common/NonCopyable.h" #include "DiscIO/Blob.h" namespace File @@ -59,13 +58,18 @@ private: ContentSource m_content_source; }; -// We do not allow copying, because it might mess up the pointers inside DiscContents -class DirectoryBlobPartition : private NonCopyable +class DirectoryBlobPartition { public: DirectoryBlobPartition() = default; DirectoryBlobPartition(const std::string& root_directory, std::optional is_wii); + // We do not allow copying, because it might mess up the pointers inside DiscContents + DirectoryBlobPartition(const DirectoryBlobPartition&) = delete; + DirectoryBlobPartition& operator=(const DirectoryBlobPartition&) = delete; + DirectoryBlobPartition(DirectoryBlobPartition&&) = default; + DirectoryBlobPartition& operator=(DirectoryBlobPartition&&) = default; + bool IsWii() const { return m_is_wii; } u64 GetDataSize() const { return m_data_size; } const std::string& GetRootDirectory() const { return m_root_directory; } @@ -103,12 +107,17 @@ private: u64 m_data_size; }; -// We do not allow copying, because it might mess up the pointers inside DiscContents -class DirectoryBlobReader : public BlobReader, private NonCopyable +class DirectoryBlobReader : public BlobReader { public: static std::unique_ptr Create(const std::string& dol_path); + // We do not allow copying, because it might mess up the pointers inside DiscContents + DirectoryBlobReader(const DirectoryBlobReader&) = delete; + DirectoryBlobReader& operator=(const DirectoryBlobReader&) = delete; + DirectoryBlobReader(DirectoryBlobReader&&) = default; + DirectoryBlobReader& operator=(DirectoryBlobReader&&) = default; + bool Read(u64 offset, u64 length, u8* buffer) override; bool SupportsReadWiiDecrypted() const override; bool ReadWiiDecrypted(u64 offset, u64 size, u8* buffer, u64 partition_offset) override; From 09f3f9b41afec593048fc24e452dc074c5eecac7 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Fri, 4 Aug 2017 23:57:12 +0200 Subject: [PATCH 2/2] 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. --- Source/Core/AudioCommon/WaveFile.h | 8 ++++++-- Source/Core/Common/CodeBlock.h | 8 ++++++-- Source/Core/Common/Common.vcxproj | 1 - Source/Core/Common/Common.vcxproj.filters | 1 - Source/Core/Common/File.h | 6 ++++-- Source/Core/Common/FileUtil.h | 1 - Source/Core/Common/Logging/LogManager.h | 8 ++++++-- Source/Core/Common/NonCopyable.h | 19 ------------------- Source/Core/Common/PcapFile.h | 3 +-- Source/Core/Core/ConfigManager.h | 8 ++++++-- Source/Core/Core/DSP/DSPCaptureLogger.h | 3 +-- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 9 +++++++-- .../Core/HW/GCMemcard/GCMemcardDirectory.h | 9 +++++++-- Source/Core/Core/HW/MMIOHandlers.h | 5 ++--- Source/Core/Core/HW/WiimoteReal/WiimoteReal.h | 8 ++++++-- Source/Core/Core/IOS/Network/Socket.h | 7 +++++-- Source/Core/DolphinQt2/Settings.h | 9 ++++++--- Source/Core/VideoBackends/OGL/SamplerCache.h | 8 ++++++-- Source/Core/VideoCommon/NativeVertexFormat.h | 8 ++++++-- 19 files changed, 75 insertions(+), 54 deletions(-) delete mode 100644 Source/Core/Common/NonCopyable.h diff --git a/Source/Core/AudioCommon/WaveFile.h b/Source/Core/AudioCommon/WaveFile.h index 7254ef82c5..45d074ace3 100644 --- a/Source/Core/AudioCommon/WaveFile.h +++ b/Source/Core/AudioCommon/WaveFile.h @@ -19,14 +19,18 @@ #include "Common/CommonTypes.h" #include "Common/File.h" -#include "Common/NonCopyable.h" -class WaveFileWriter : NonCopyable +class WaveFileWriter { public: 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); void Stop(); diff --git a/Source/Core/Common/CodeBlock.h b/Source/Core/Common/CodeBlock.h index cd933647ed..9cbe3ffc5b 100644 --- a/Source/Core/Common/CodeBlock.h +++ b/Source/Core/Common/CodeBlock.h @@ -10,7 +10,6 @@ #include "Common/Assert.h" #include "Common/CommonTypes.h" #include "Common/MemoryUtil.h" -#include "Common/NonCopyable.h" // Everything that needs to generate code should inherit from this. // You get memory management for free, plus, you can use all emitter functions without @@ -18,7 +17,7 @@ // Example implementation: // class JIT : public CodeBlock {} template -class CodeBlock : public T, NonCopyable +class CodeBlock : public T { private: // A privately used function to set the executable RAM space to something invalid. @@ -37,11 +36,16 @@ protected: std::vector m_children; public: + CodeBlock() = default; virtual ~CodeBlock() { if (region) FreeCodeSpace(); } + CodeBlock(const CodeBlock&) = delete; + CodeBlock& operator=(const CodeBlock&) = delete; + CodeBlock(CodeBlock&&) = delete; + CodeBlock& operator=(CodeBlock&&) = delete; // Call this before you generate any code. void AllocCodeSpace(size_t size) diff --git a/Source/Core/Common/Common.vcxproj b/Source/Core/Common/Common.vcxproj index 651808e5f7..d81106f512 100644 --- a/Source/Core/Common/Common.vcxproj +++ b/Source/Core/Common/Common.vcxproj @@ -136,7 +136,6 @@ - diff --git a/Source/Core/Common/Common.vcxproj.filters b/Source/Core/Common/Common.vcxproj.filters index c7ceb7adcb..a991205327 100644 --- a/Source/Core/Common/Common.vcxproj.filters +++ b/Source/Core/Common/Common.vcxproj.filters @@ -237,7 +237,6 @@ GL\GLInterface - diff --git a/Source/Core/Common/File.h b/Source/Core/Common/File.h index 3c9504b58a..b92cc309a5 100644 --- a/Source/Core/Common/File.h +++ b/Source/Core/Common/File.h @@ -9,14 +9,13 @@ #include #include "Common/CommonTypes.h" -#include "Common/NonCopyable.h" namespace File { // simple wrapper for cstdlib file functions to // hopefully will make error checking easier // and make forgetting an fclose() harder -class IOFile : public NonCopyable +class IOFile { public: IOFile(); @@ -25,6 +24,9 @@ public: ~IOFile(); + IOFile(const IOFile&) = delete; + IOFile& operator=(const IOFile&) = delete; + IOFile(IOFile&& other) noexcept; IOFile& operator=(IOFile&& other) noexcept; diff --git a/Source/Core/Common/FileUtil.h b/Source/Core/Common/FileUtil.h index d8f1c660eb..398abc748e 100644 --- a/Source/Core/Common/FileUtil.h +++ b/Source/Core/Common/FileUtil.h @@ -12,7 +12,6 @@ #include #include "Common/CommonTypes.h" -#include "Common/NonCopyable.h" #ifdef _WIN32 #include "Common/StringUtil.h" diff --git a/Source/Core/Common/Logging/LogManager.h b/Source/Core/Common/Logging/LogManager.h index 153e4d0b0f..0903116cc6 100644 --- a/Source/Core/Common/Logging/LogManager.h +++ b/Source/Core/Common/Logging/LogManager.h @@ -9,7 +9,6 @@ #include "Common/BitSet.h" #include "Common/Logging/Log.h" -#include "Common/NonCopyable.h" // pure virtual interface class LogListener @@ -28,7 +27,7 @@ public: }; }; -class LogManager : NonCopyable +class LogManager { public: static LogManager* GetInstance(); @@ -66,6 +65,11 @@ private: LogManager(); ~LogManager(); + LogManager(const LogManager&) = delete; + LogManager& operator=(const LogManager&) = delete; + LogManager(LogManager&&) = delete; + LogManager& operator=(LogManager&&) = delete; + LogTypes::LOG_LEVELS m_level; std::array m_log{}; std::array m_listeners{}; diff --git a/Source/Core/Common/NonCopyable.h b/Source/Core/Common/NonCopyable.h deleted file mode 100644 index 2325156d3d..0000000000 --- a/Source/Core/Common/NonCopyable.h +++ /dev/null @@ -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; -}; diff --git a/Source/Core/Common/PcapFile.h b/Source/Core/Common/PcapFile.h index b37e0e845f..4e33b25802 100644 --- a/Source/Core/Common/PcapFile.h +++ b/Source/Core/Common/PcapFile.h @@ -18,9 +18,8 @@ #include "Common/CommonTypes.h" #include "Common/File.h" -#include "Common/NonCopyable.h" -class PCAP final : public NonCopyable +class PCAP final { public: // Takes ownership of the file object. Assumes the file object is already diff --git a/Source/Core/Core/ConfigManager.h b/Source/Core/Core/ConfigManager.h index edaf5b3a5e..247cbd659a 100644 --- a/Source/Core/Core/ConfigManager.h +++ b/Source/Core/Core/ConfigManager.h @@ -12,7 +12,6 @@ #include #include "Common/IniFile.h" -#include "Common/NonCopyable.h" #include "Core/HW/EXI/EXI_Device.h" #include "Core/HW/SI/SI_Device.h" #include "Core/TitleDatabase.h" @@ -52,7 +51,7 @@ enum GPUDeterminismMode struct BootParameters; -struct SConfig : NonCopyable +struct SConfig { // Wii Devices bool m_WiiSDCard; @@ -318,6 +317,11 @@ struct SConfig : NonCopyable bool m_SSLDumpRootCA; bool m_SSLDumpPeerCert; + SConfig(const SConfig&) = delete; + SConfig& operator=(const SConfig&) = delete; + SConfig(SConfig&&) = delete; + SConfig& operator=(SConfig&&) = delete; + // Save settings void SaveSettings(); diff --git a/Source/Core/Core/DSP/DSPCaptureLogger.h b/Source/Core/Core/DSP/DSPCaptureLogger.h index a4335f3322..07ac068349 100644 --- a/Source/Core/Core/DSP/DSPCaptureLogger.h +++ b/Source/Core/Core/DSP/DSPCaptureLogger.h @@ -8,7 +8,6 @@ #include #include "Common/CommonTypes.h" -#include "Common/NonCopyable.h" class PCAP; @@ -51,7 +50,7 @@ public: // A capture logger implementation that logs to PCAP files in a custom // packet-based format. -class PCAPDSPCaptureLogger final : public DSPCaptureLogger, NonCopyable +class PCAPDSPCaptureLogger final : public DSPCaptureLogger { public: // Automatically creates a writeable file (truncate existing file). diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index a7dfffa0ae..1e7bea5027 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -9,7 +9,6 @@ #include "Common/CommonTypes.h" #include "Common/NandPaths.h" -#include "Common/NonCopyable.h" #include "Common/Swap.h" #include "Common/Timer.h" @@ -295,7 +294,7 @@ public: std::string m_filename; }; -class GCMemcard : NonCopyable +class GCMemcard { private: bool m_valid; @@ -317,6 +316,12 @@ private: public: explicit GCMemcard(const std::string& fileName, bool forceCreation = 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 IsShiftJIS() const; bool Save(); diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.h b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.h index 4d7de844a4..363c326a8a 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.h @@ -10,19 +10,24 @@ #include #include "Common/Event.h" -#include "Common/NonCopyable.h" #include "Core/HW/GCMemcard/GCMemcard.h" // Uncomment this to write the system data of the memorycard from directory to disc //#define _WRITE_MC_HEADER 1 void MigrateFromMemcardFile(const std::string& directory_name, int card_index); -class GCMemcardDirectory : public MemoryCardBase, NonCopyable +class GCMemcardDirectory : public MemoryCardBase { public: GCMemcardDirectory(const std::string& directory, int slot, u16 size_mbits, bool shift_jis, int game_id); ~GCMemcardDirectory(); + + GCMemcardDirectory(const GCMemcardDirectory&) = delete; + GCMemcardDirectory& operator=(const GCMemcardDirectory&) = delete; + GCMemcardDirectory(GCMemcardDirectory&&) = default; + GCMemcardDirectory& operator=(GCMemcardDirectory&&) = default; + void FlushToFile(); void FlushThread(); s32 Read(u32 src_address, s32 length, u8* dest_address) override; diff --git a/Source/Core/Core/HW/MMIOHandlers.h b/Source/Core/Core/HW/MMIOHandlers.h index a406320a3b..40d71753df 100644 --- a/Source/Core/Core/HW/MMIOHandlers.h +++ b/Source/Core/Core/HW/MMIOHandlers.h @@ -8,7 +8,6 @@ #include #include "Common/CommonTypes.h" -#include "Common/NonCopyable.h" // 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 @@ -119,7 +118,7 @@ public: // inlinable, we need to provide some of the implementation of these two // classes here and can't just use a forward declaration. template -class ReadHandler : public NonCopyable +class ReadHandler { public: ReadHandler(); @@ -155,7 +154,7 @@ private: std::function m_ReadFunc; }; template -class WriteHandler : public NonCopyable +class WriteHandler { public: WriteHandler(); diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h index 2c213a9dec..98754a8ae3 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h @@ -14,7 +14,6 @@ #include "Common/Event.h" #include "Common/FifoQueue.h" #include "Common/Flag.h" -#include "Common/NonCopyable.h" #include "Core/HW/Wiimote.h" #include "Core/HW/WiimoteCommon/WiimoteConstants.h" #include "Core/HW/WiimoteCommon/WiimoteHid.h" @@ -24,9 +23,14 @@ class PointerWrap; namespace WiimoteReal { -class Wiimote : NonCopyable +class Wiimote { public: + Wiimote(const Wiimote&) = delete; + Wiimote& operator=(const Wiimote&) = delete; + Wiimote(Wiimote&&) = default; + Wiimote& operator=(Wiimote&&) = default; + virtual ~Wiimote() {} // This needs to be called in derived destructors! void Shutdown(); diff --git a/Source/Core/Core/IOS/Network/Socket.h b/Source/Core/Core/IOS/Network/Socket.h index aa503c1f75..c02cd2ee43 100644 --- a/Source/Core/Core/IOS/Network/Socket.h +++ b/Source/Core/Core/IOS/Network/Socket.h @@ -50,7 +50,6 @@ typedef struct pollfd pollfd_t; #include "Common/CommonTypes.h" #include "Common/Logging/Log.h" -#include "Common/NonCopyable.h" #include "Core/HW/Memmap.h" #include "Core/IOS/IOS.h" #include "Core/IOS/Network/IP/Top.h" @@ -207,7 +206,7 @@ public: void operator=(WiiSocket const&) = delete; }; -class WiiSockMan : public ::NonCopyable +class WiiSockMan { public: static s32 GetNetErrorCode(s32 ret, const char* caller, bool isRW); @@ -249,6 +248,10 @@ public: private: WiiSockMan() = default; + WiiSockMan(const WiiSockMan&) = delete; + WiiSockMan& operator=(const WiiSockMan&) = delete; + WiiSockMan(WiiSockMan&&) = delete; + WiiSockMan& operator=(WiiSockMan&&) = delete; std::unordered_map WiiSockets; s32 errno_last; diff --git a/Source/Core/DolphinQt2/Settings.h b/Source/Core/DolphinQt2/Settings.h index d3d800668b..54bf446153 100644 --- a/Source/Core/DolphinQt2/Settings.h +++ b/Source/Core/DolphinQt2/Settings.h @@ -9,8 +9,6 @@ #include #include -#include "Common/NonCopyable.h" - #include "Core/NetPlayClient.h" #include "Core/NetPlayServer.h" @@ -23,11 +21,16 @@ class GameListModel; class InputConfig; // UI settings to be stored in the config directory. -class Settings final : public QObject, NonCopyable +class Settings final : public QObject { Q_OBJECT public: + Settings(const Settings&) = delete; + Settings& operator=(const Settings&) = delete; + Settings(Settings&&) = delete; + Settings& operator=(Settings&&) = delete; + static Settings& Instance(); // UI diff --git a/Source/Core/VideoBackends/OGL/SamplerCache.h b/Source/Core/VideoBackends/OGL/SamplerCache.h index 232f253949..c6bd76dae1 100644 --- a/Source/Core/VideoBackends/OGL/SamplerCache.h +++ b/Source/Core/VideoBackends/OGL/SamplerCache.h @@ -9,17 +9,21 @@ #include "Common/CommonTypes.h" #include "Common/GL/GLUtil.h" -#include "Common/NonCopyable.h" #include "VideoBackends/OGL/Render.h" namespace OGL { -class SamplerCache : NonCopyable +class SamplerCache { public: 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 Clear(); void BindNearestSampler(int stage); diff --git a/Source/Core/VideoCommon/NativeVertexFormat.h b/Source/Core/VideoCommon/NativeVertexFormat.h index f03de05cc6..ac75f68652 100644 --- a/Source/Core/VideoCommon/NativeVertexFormat.h +++ b/Source/Core/VideoCommon/NativeVertexFormat.h @@ -9,7 +9,6 @@ #include "Common/CommonTypes.h" #include "Common/Hash.h" -#include "Common/NonCopyable.h" // m_components enum @@ -102,10 +101,15 @@ struct hash // 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. -class NativeVertexFormat : NonCopyable +class NativeVertexFormat { public: 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; } const PortableVertexDeclaration& GetVertexDeclaration() const { return vtx_decl; } protected: