From 7ef4d7762f0ad1166e1e0280f7a1ec18a5d9fb4d Mon Sep 17 00:00:00 2001 From: skdltmxn Date: Tue, 12 Jan 2021 06:14:15 +0900 Subject: [PATCH] Refactor to avoid unnecessary copies (#13736) * Refactor to avoid unnecessary copies * Fix dangling references --- src/openrct2-ui/UiContext.Win32.cpp | 2 +- src/openrct2-ui/scripting/CustomListView.cpp | 2 +- src/openrct2-ui/scripting/CustomListView.h | 2 +- src/openrct2-ui/scripting/CustomWindow.cpp | 6 ++-- src/openrct2-ui/scripting/ScUi.hpp | 4 +-- src/openrct2-ui/windows/Changelog.cpp | 2 +- src/openrct2/ParkImporter.h | 2 +- src/openrct2/ReplayManager.cpp | 2 +- src/openrct2/actions/GameAction.h | 2 +- src/openrct2/core/Collections.hpp | 8 +++--- src/openrct2/core/FileIndex.hpp | 2 +- src/openrct2/core/JobPool.cpp | 2 +- src/openrct2/core/MemoryStream.cpp | 30 ++++++++++++++------ src/openrct2/interface/Viewport.cpp | 2 +- src/openrct2/network/NetworkBase.cpp | 2 +- src/openrct2/network/NetworkUser.cpp | 2 +- src/openrct2/object/Object.cpp | 4 +-- src/openrct2/object/Object.h | 2 +- src/openrct2/object/ObjectRepository.cpp | 2 +- src/openrct2/platform/Crash.cpp | 2 +- src/openrct2/platform/Platform.Linux.cpp | 2 +- src/openrct2/scripting/ScriptEngine.cpp | 2 +- test/tests/CryptTests.cpp | 6 ++-- 23 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/openrct2-ui/UiContext.Win32.cpp b/src/openrct2-ui/UiContext.Win32.cpp index 9ffc58d766..8a377b5eec 100644 --- a/src/openrct2-ui/UiContext.Win32.cpp +++ b/src/openrct2-ui/UiContext.Win32.cpp @@ -235,7 +235,7 @@ namespace OpenRCT2::Ui static std::wstring GetFilterString(const std::vector filters) { std::wstringstream filtersb; - for (auto filter : filters) + for (const auto& filter : filters) { filtersb << String::ToWideChar(filter.Name) << '\0' << String::ToWideChar(filter.Pattern) << '\0'; } diff --git a/src/openrct2-ui/scripting/CustomListView.cpp b/src/openrct2-ui/scripting/CustomListView.cpp index 7d7304553b..40605cb418 100644 --- a/src/openrct2-ui/scripting/CustomListView.cpp +++ b/src/openrct2-ui/scripting/CustomListView.cpp @@ -278,7 +278,7 @@ void CustomListView::SetItems(const std::vector& items, bool initi void CustomListView::SetItems(std::vector&& items, bool initialising) { - Items = items; + Items = std::move(items); SortItems(0, ColumnSortOrder::None); if (!initialising) { diff --git a/src/openrct2-ui/scripting/CustomListView.h b/src/openrct2-ui/scripting/CustomListView.h index 095be89f5d..093148cd58 100644 --- a/src/openrct2-ui/scripting/CustomListView.h +++ b/src/openrct2-ui/scripting/CustomListView.h @@ -61,7 +61,7 @@ namespace OpenRCT2::Ui::Windows Cells.emplace_back(text); } explicit ListViewItem(std::vector&& cells) - : Cells(cells) + : Cells(std::move(cells)) { } }; diff --git a/src/openrct2-ui/scripting/CustomWindow.cpp b/src/openrct2-ui/scripting/CustomWindow.cpp index a2c72de35e..4933586301 100644 --- a/src/openrct2-ui/scripting/CustomWindow.cpp +++ b/src/openrct2-ui/scripting/CustomWindow.cpp @@ -703,7 +703,7 @@ namespace OpenRCT2::Ui::Windows const auto& customInfo = GetInfo(w); const auto& tabs = customInfo.Desc.Tabs; size_t tabIndex = 0; - for (auto tab : tabs) + for (const auto& tab : tabs) { auto widgetIndex = static_cast(WIDX_TAB_0 + tabIndex); auto widget = &w->widgets[widgetIndex]; @@ -1244,11 +1244,11 @@ namespace OpenRCT2::Ui::Windows rct_window* FindCustomWindowByClassification(std::string_view classification) { - for (auto w : g_window_list) + for (const auto& w : g_window_list) { if (w->classification == WC_CUSTOM) { - auto& customInfo = GetInfo(w.get()); + const auto& customInfo = GetInfo(w.get()); if (customInfo.Desc.Classification == classification) { return w.get(); diff --git a/src/openrct2-ui/scripting/ScUi.hpp b/src/openrct2-ui/scripting/ScUi.hpp index 100bbc4258..ee22d43845 100644 --- a/src/openrct2-ui/scripting/ScUi.hpp +++ b/src/openrct2-ui/scripting/ScUi.hpp @@ -160,7 +160,7 @@ namespace OpenRCT2::Scripting { auto index = a.as_int(); auto i = 0; - for (auto w : g_window_list) + for (const auto& w : g_window_list) { if (i == index) { @@ -171,7 +171,7 @@ namespace OpenRCT2::Scripting } else if (a.type() == DukValue::Type::STRING) { - auto classification = a.as_string(); + const auto& classification = a.as_string(); auto w = FindCustomWindowByClassification(classification); if (w != nullptr) { diff --git a/src/openrct2-ui/windows/Changelog.cpp b/src/openrct2-ui/windows/Changelog.cpp index d37f0a0eaa..5fd532d283 100644 --- a/src/openrct2-ui/windows/Changelog.cpp +++ b/src/openrct2-ui/windows/Changelog.cpp @@ -219,7 +219,7 @@ static void window_changelog_scrollpaint(rct_window* w, rct_drawpixelinfo* dpi, const int32_t lineHeight = font_get_line_height(gCurrentFontSpriteBase); ScreenCoordsXY screenCoords(3, 3 - lineHeight); - for (auto line : _changelogLines) + for (const auto& line : _changelogLines) { screenCoords.y += lineHeight; if (screenCoords.y + lineHeight < dpi->y || screenCoords.y >= dpi->y + dpi->height) diff --git a/src/openrct2/ParkImporter.h b/src/openrct2/ParkImporter.h index 54b9bc447b..1edcc8b146 100644 --- a/src/openrct2/ParkImporter.h +++ b/src/openrct2/ParkImporter.h @@ -71,7 +71,7 @@ public: std::vector const MissingObjects; explicit ObjectLoadException(std::vector&& missingObjects) - : MissingObjects(missingObjects) + : MissingObjects(std::move(missingObjects)) { } }; diff --git a/src/openrct2/ReplayManager.cpp b/src/openrct2/ReplayManager.cpp index f0450f4e4b..f4730ae25d 100644 --- a/src/openrct2/ReplayManager.cpp +++ b/src/openrct2/ReplayManager.cpp @@ -147,7 +147,7 @@ namespace OpenRCT2 void AddChecksum(uint32_t tick, rct_sprite_checksum&& checksum) { - _currentRecording->checksums.emplace_back(std::make_pair(tick, checksum)); + _currentRecording->checksums.emplace_back(std::make_pair(tick, std::move(checksum))); } // Function runs each Tick. diff --git a/src/openrct2/actions/GameAction.h b/src/openrct2/actions/GameAction.h index 74a8f8131c..04ac4b2b36 100644 --- a/src/openrct2/actions/GameAction.h +++ b/src/openrct2/actions/GameAction.h @@ -41,7 +41,7 @@ public: } StringVariant(std::string&& s) - : String(s) + : String(std::move(s)) { } diff --git a/src/openrct2/core/Collections.hpp b/src/openrct2/core/Collections.hpp index 70e419684c..a9cb9ec818 100644 --- a/src/openrct2/core/Collections.hpp +++ b/src/openrct2/core/Collections.hpp @@ -37,10 +37,10 @@ namespace Collections } template - static size_t IndexOf(TCollection& collection, TItem needle, TComparer comparer) + static size_t IndexOf(const TCollection& collection, TItem needle, TComparer comparer) { size_t index = 0; - for (TItem item : collection) + for (const auto& item : collection) { if (comparer(item, needle)) { @@ -51,10 +51,10 @@ namespace Collections return SIZE_MAX; } - template static size_t IndexOf(TCollection& collection, TPred predicate) + template static size_t IndexOf(const TCollection& collection, TPred predicate) { size_t index = 0; - for (auto item : collection) + for (const auto& item : collection) { if (predicate(item)) { diff --git a/src/openrct2/core/FileIndex.hpp b/src/openrct2/core/FileIndex.hpp index 5a62ccdeff..033a286b14 100644 --- a/src/openrct2/core/FileIndex.hpp +++ b/src/openrct2/core/FileIndex.hpp @@ -233,7 +233,7 @@ private: jobPool.Join(reportProgress); - for (auto&& itr : containers) + for (const auto& itr : containers) { allItems.insert(allItems.end(), itr.begin(), itr.end()); } diff --git a/src/openrct2/core/JobPool.cpp b/src/openrct2/core/JobPool.cpp index f979467bb4..117a8d68a0 100644 --- a/src/openrct2/core/JobPool.cpp +++ b/src/openrct2/core/JobPool.cpp @@ -35,7 +35,7 @@ JobPool::~JobPool() _condPending.notify_all(); } - for (auto&& th : _threads) + for (auto& th : _threads) { assert(th.joinable() != false); th.join(); diff --git a/src/openrct2/core/MemoryStream.cpp b/src/openrct2/core/MemoryStream.cpp index 326e1232a6..c305403bdc 100644 --- a/src/openrct2/core/MemoryStream.cpp +++ b/src/openrct2/core/MemoryStream.cpp @@ -52,8 +52,16 @@ namespace OpenRCT2 } MemoryStream::MemoryStream(MemoryStream&& mv) noexcept + : _access(mv._access) + , _dataCapacity(mv._dataCapacity) + , _dataSize(mv._dataSize) + , _data(mv._data) + , _position(mv._position) { - *this = std::move(mv); + mv._data = nullptr; + mv._position = nullptr; + mv._dataCapacity = 0; + mv._dataSize = 0; } MemoryStream::~MemoryStream() @@ -69,15 +77,19 @@ namespace OpenRCT2 MemoryStream& MemoryStream::operator=(MemoryStream&& mv) noexcept { - _access = mv._access; - _dataCapacity = mv._dataCapacity; - _data = mv._data; - _position = mv._position; + if (this != &mv) + { + _access = mv._access; + _dataCapacity = mv._dataCapacity; + _data = mv._data; + _dataSize = mv._dataSize; + _position = mv._position; - mv._data = nullptr; - mv._position = nullptr; - mv._dataCapacity = 0; - mv._dataSize = 0; + mv._data = nullptr; + mv._position = nullptr; + mv._dataCapacity = 0; + mv._dataSize = 0; + } return *this; } diff --git a/src/openrct2/interface/Viewport.cpp b/src/openrct2/interface/Viewport.cpp index 24ddca5884..fb85d02eb9 100644 --- a/src/openrct2/interface/Viewport.cpp +++ b/src/openrct2/interface/Viewport.cpp @@ -1020,7 +1020,7 @@ void viewport_paint( _paintJobs->Join(); } - for (auto&& column : columns) + for (auto column : columns) { viewport_paint_column(column); } diff --git a/src/openrct2/network/NetworkBase.cpp b/src/openrct2/network/NetworkBase.cpp index dcc92f51eb..11b5f0337b 100644 --- a/src/openrct2/network/NetworkBase.cpp +++ b/src/openrct2/network/NetworkBase.cpp @@ -1830,7 +1830,7 @@ void NetworkBase::ProcessPlayerList() std::vector newPlayers; std::vector removedPlayers; - for (auto&& pendingPlayer : itPending->second.players) + for (const auto& pendingPlayer : itPending->second.players) { activePlayerIds.push_back(pendingPlayer.Id); diff --git a/src/openrct2/network/NetworkUser.cpp b/src/openrct2/network/NetworkUser.cpp index b1275b3810..456262e814 100644 --- a/src/openrct2/network/NetworkUser.cpp +++ b/src/openrct2/network/NetworkUser.cpp @@ -210,7 +210,7 @@ const NetworkUser* NetworkUserManager::GetUserByHash(const std::string& hash) co const NetworkUser* NetworkUserManager::GetUserByName(const std::string& name) const { - for (auto kvp : _usersByHash) + for (const auto& kvp : _usersByHash) { const NetworkUser* networkUser = kvp.second; if (String::Equals(name.c_str(), networkUser->Name.c_str(), true)) diff --git a/src/openrct2/object/Object.cpp b/src/openrct2/object/Object.cpp index a0d6c984bf..a037985993 100644 --- a/src/openrct2/object/Object.cpp +++ b/src/openrct2/object/Object.cpp @@ -141,9 +141,9 @@ const std::vector& Object::GetAuthors() const return _authors; } -void Object::SetAuthors(const std::vector&& authors) +void Object::SetAuthors(std::vector&& authors) { - _authors = authors; + _authors = std::move(authors); } std::optional rct_object_entry::GetSceneryType() const diff --git a/src/openrct2/object/Object.h b/src/openrct2/object/Object.h index 8236f0fa0e..ae045a02bd 100644 --- a/src/openrct2/object/Object.h +++ b/src/openrct2/object/Object.h @@ -319,7 +319,7 @@ public: void SetSourceGames(const std::vector& sourceGames); const std::vector& GetAuthors() const; - void SetAuthors(const std::vector&& authors); + void SetAuthors(std::vector&& authors); const ImageTable& GetImageTable() const { diff --git a/src/openrct2/object/ObjectRepository.cpp b/src/openrct2/object/ObjectRepository.cpp index 8812769156..63bec03893 100644 --- a/src/openrct2/object/ObjectRepository.cpp +++ b/src/openrct2/object/ObjectRepository.cpp @@ -399,7 +399,7 @@ private: void AddItems(const std::vector& items) { size_t numConflicts = 0; - for (auto item : items) + for (const auto& item : items) { if (!AddItem(item)) { diff --git a/src/openrct2/platform/Crash.cpp b/src/openrct2/platform/Crash.cpp index 28ddc40536..251ffe8f77 100644 --- a/src/openrct2/platform/Crash.cpp +++ b/src/openrct2/platform/Crash.cpp @@ -59,7 +59,7 @@ const wchar_t* _wszArchitecture = WSZ(OPENRCT2_ARCHITECTURE); // https://documentation.backtrace.io/product_integration_minidump_breakpad/ static bool UploadMinidump(const std::map& files, int& error, std::wstring& response) { - for (auto file : files) + for (const auto& file : files) { wprintf(L"files[%s] = %s\n", file.first.c_str(), file.second.c_str()); } diff --git a/src/openrct2/platform/Platform.Linux.cpp b/src/openrct2/platform/Platform.Linux.cpp index 9daa8980cc..6c78d94254 100644 --- a/src/openrct2/platform/Platform.Linux.cpp +++ b/src/openrct2/platform/Platform.Linux.cpp @@ -104,7 +104,7 @@ namespace Platform "/var/lib/openrct2", "/usr/share/openrct2", }; - for (auto prefix : prefixes) + for (const auto& prefix : prefixes) { for (auto searchLocation : SearchLocations) { diff --git a/src/openrct2/scripting/ScriptEngine.cpp b/src/openrct2/scripting/ScriptEngine.cpp index 4327acdb69..8512b41f79 100644 --- a/src/openrct2/scripting/ScriptEngine.cpp +++ b/src/openrct2/scripting/ScriptEngine.cpp @@ -492,7 +492,7 @@ void ScriptEngine::StopPlugin(std::shared_ptr plugin) RemoveIntervals(plugin); RemoveSockets(plugin); _hookEngine.UnsubscribeAll(plugin); - for (auto callback : _pluginStoppedSubscriptions) + for (const auto& callback : _pluginStoppedSubscriptions) { callback(plugin); } diff --git a/test/tests/CryptTests.cpp b/test/tests/CryptTests.cpp index 90314ee674..e2afd9a6ba 100644 --- a/test/tests/CryptTests.cpp +++ b/test/tests/CryptTests.cpp @@ -68,7 +68,7 @@ TEST_F(CryptTests, SHA1_Multiple) }; auto alg = Crypt::CreateSHA1(); - for (auto s : input) + for (const auto& s : input) { alg->Update(s.data(), s.size()); } @@ -99,7 +99,7 @@ TEST_F(CryptTests, SHA1_Many) "This park is really clean and tidy", "This balloon from Balloon Stall 1 is really good value", }; - for (auto s : inputA) + for (const auto& s : inputA) { alg->Update(s.data(), s.size()); } @@ -113,7 +113,7 @@ TEST_F(CryptTests, SHA1_Many) "This park is really clean and tidy", "Merry-go-round 2 looks too intense for me", }; - for (auto s : inputB) + for (const auto& s : inputB) { alg->Update(s.data(), s.size()); }