From 8360fab18a99a1e8097f37137324a783e76cd25a Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Wed, 16 Oct 2024 00:16:43 +0100 Subject: [PATCH] Codechange: Remove CCountedPtr. This was originally generic and used by YAPF, but now it is used only by script objects. CCountedPtr provided much more (untested) functionality than used. ScriptObjectRef already exists for script objects and does the same thing, so use this instead. --- src/misc/CMakeLists.txt | 2 - src/misc/countedobj.cpp | 35 ----- src/misc/countedptr.hpp | 213 -------------------------- src/newgrf_config.h | 1 - src/script/api/script_basestation.cpp | 2 +- src/script/api/script_company.cpp | 4 +- src/script/api/script_goal.cpp | 8 +- src/script/api/script_group.cpp | 2 +- src/script/api/script_industry.cpp | 4 +- src/script/api/script_league.cpp | 14 +- src/script/api/script_news.cpp | 2 +- src/script/api/script_object.cpp | 15 ++ src/script/api/script_object.hpp | 24 ++- src/script/api/script_sign.cpp | 4 +- src/script/api/script_story_page.cpp | 8 +- src/script/api/script_town.cpp | 6 +- src/script/api/script_vehicle.cpp | 2 +- src/script/script_info.hpp | 2 +- 18 files changed, 66 insertions(+), 282 deletions(-) delete mode 100644 src/misc/countedobj.cpp delete mode 100644 src/misc/countedptr.hpp diff --git a/src/misc/CMakeLists.txt b/src/misc/CMakeLists.txt index f088dd5f8d..f45fa06960 100644 --- a/src/misc/CMakeLists.txt +++ b/src/misc/CMakeLists.txt @@ -1,7 +1,5 @@ add_files( binaryheap.hpp - countedobj.cpp - countedptr.hpp dbg_helpers.cpp dbg_helpers.h endian_buffer.hpp diff --git a/src/misc/countedobj.cpp b/src/misc/countedobj.cpp deleted file mode 100644 index 563ae3f4bb..0000000000 --- a/src/misc/countedobj.cpp +++ /dev/null @@ -1,35 +0,0 @@ -/* - * This file is part of OpenTTD. - * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. - * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . - */ - -/** @file countedobj.cpp Support for reference counted objects. */ - -#include "../stdafx.h" - -#include "countedptr.hpp" - -#include "../safeguards.h" - -int32_t SimpleCountedObject::AddRef() -{ - return ++m_ref_cnt; -} - -int32_t SimpleCountedObject::Release() -{ - int32_t res = --m_ref_cnt; - assert(res >= 0); - if (res == 0) { - try { - FinalRelease(); // may throw, for example ScriptTest/ExecMode - } catch (...) { - delete this; - throw; - } - delete this; - } - return res; -} diff --git a/src/misc/countedptr.hpp b/src/misc/countedptr.hpp deleted file mode 100644 index 625b124200..0000000000 --- a/src/misc/countedptr.hpp +++ /dev/null @@ -1,213 +0,0 @@ -/* - * This file is part of OpenTTD. - * OpenTTD is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 2. - * OpenTTD is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see . - */ - -/** @file countedptr.hpp CCountedPtr - smart pointer implementation. */ - -#ifndef COUNTEDPTR_HPP -#define COUNTEDPTR_HPP - -/** - * CCountedPtr - simple reference counting smart pointer. - * - * One of the standard ways how to maintain object's lifetime. - * - * See http://ootips.org/yonat/4dev/smart-pointers.html for more - * general info about smart pointers. - * - * This class implements ref-counted pointer for objects/interfaces that - * support AddRef() and Release() methods. - */ -template -class CCountedPtr { - /** redefine the template argument to make it visible for derived classes */ -public: - typedef Tcls_ Tcls; - -protected: - /** here we hold our pointer to the target */ - Tcls *m_pT; - -public: - /** default (nullptr) construct or construct from a raw pointer */ - inline CCountedPtr(Tcls *pObj = nullptr) : m_pT(pObj) - { - AddRef(); - } - - /** copy constructor (invoked also when initializing from another smart ptr) */ - inline CCountedPtr(const CCountedPtr &src) : m_pT(src.m_pT) - { - AddRef(); - } - - /** destructor releasing the reference */ - inline ~CCountedPtr() - { - Release(); - } - -protected: - /** add one ref to the underlying object */ - inline void AddRef() - { - if (m_pT != nullptr) m_pT->AddRef(); - } - -public: - /** release smart pointer (and decrement ref count) if not null */ - inline void Release() - { - if (m_pT != nullptr) { - Tcls *pT = m_pT; - m_pT = nullptr; - pT->Release(); - } - } - - /** dereference of smart pointer - const way */ - inline const Tcls *operator->() const - { - assert(m_pT != nullptr); - return m_pT; - } - - /** dereference of smart pointer - non const way */ - inline Tcls *operator->() - { - assert(m_pT != nullptr); - return m_pT; - } - - /** raw pointer casting operator - const way */ - inline operator const Tcls*() const - { - assert(m_pT == nullptr); - return m_pT; - } - - /** raw pointer casting operator - non-const way */ - inline operator Tcls*() - { - return m_pT; - } - - /** operator & to support output arguments */ - inline Tcls** operator&() - { - assert(m_pT == nullptr); - return &m_pT; - } - - /** assignment operator from raw ptr */ - inline CCountedPtr& operator=(Tcls *pT) - { - Assign(pT); - return *this; - } - - /** assignment operator from another smart ptr */ - inline CCountedPtr& operator=(const CCountedPtr &src) - { - Assign(src.m_pT); - return *this; - } - - /** assignment operator helper */ - inline void Assign(Tcls *pT); - - /** one way how to test for nullptr value */ - inline bool IsNull() const - { - return m_pT == nullptr; - } - - /** assign pointer w/o incrementing ref count */ - inline void Attach(Tcls *pT) - { - Release(); - m_pT = pT; - } - - /** detach pointer w/o decrementing ref count */ - inline Tcls *Detach() - { - Tcls *pT = m_pT; - m_pT = nullptr; - return pT; - } -}; - -template -inline void CCountedPtr::Assign(Tcls *pT) -{ - /* if they are the same, we do nothing */ - if (pT != m_pT) { - if (pT != nullptr) pT->AddRef(); // AddRef new pointer if any - Tcls *pTold = m_pT; // save original ptr - m_pT = pT; // update m_pT to new value - if (pTold != nullptr) pTold->Release(); // release old ptr if any - } -} - -/** - * Adapter wrapper for CCountedPtr like classes that can't be used directly by stl - * collections as item type. For example CCountedPtr has overloaded operator & which - * prevents using CCountedPtr in stl collections (i.e. std::list >) - */ -template struct AdaptT { - T m_t; - - /** construct by wrapping the given object */ - AdaptT(const T &t) - : m_t(t) - {} - - /** assignment operator */ - T& operator = (const T &t) - { - m_t = t; - return t; - } - - /** type-cast operator (used when AdaptT is used instead of T) */ - operator T& () - { - return m_t; - } - - /** const type-cast operator (used when AdaptT is used instead of const T) */ - operator const T& () const - { - return m_t; - } -}; - -/** - * Simple counted object. Use it as base of your struct/class if you want to use - * basic reference counting. Your struct/class will destroy and free itself when - * last reference to it is released (using Release() method). The initial reference - * count (when it is created) is zero (don't forget AddRef() at least one time if - * not using CCountedPtr. - * - * @see misc/countedobj.cpp for implementation. - */ -struct SimpleCountedObject { - int32_t m_ref_cnt; - - SimpleCountedObject() - : m_ref_cnt(0) - {} - - virtual ~SimpleCountedObject() - {} - - virtual int32_t AddRef(); - virtual int32_t Release(); - virtual void FinalRelease() {}; -}; - -#endif /* COUNTEDPTR_HPP */ diff --git a/src/newgrf_config.h b/src/newgrf_config.h index b664bb8cb1..aad86f7d33 100644 --- a/src/newgrf_config.h +++ b/src/newgrf_config.h @@ -12,7 +12,6 @@ #include "strings_type.h" #include "core/alloc_type.hpp" -#include "misc/countedptr.hpp" #include "fileio_type.h" #include "textfile_type.h" #include "newgrf_text.h" diff --git a/src/script/api/script_basestation.cpp b/src/script/api/script_basestation.cpp index 2c450937b5..7aff814c6a 100644 --- a/src/script/api/script_basestation.cpp +++ b/src/script/api/script_basestation.cpp @@ -37,7 +37,7 @@ /* static */ bool ScriptBaseStation::SetName(StationID station_id, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceCompanyModeValid(false); EnforcePrecondition(false, IsValidBaseStation(station_id)); diff --git a/src/script/api/script_company.cpp b/src/script/api/script_company.cpp index 842a51c9fc..340b31918b 100644 --- a/src/script/api/script_company.cpp +++ b/src/script/api/script_company.cpp @@ -46,7 +46,7 @@ /* static */ bool ScriptCompany::SetName(Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceCompanyModeValid(false); EnforcePrecondition(false, name != nullptr); @@ -68,7 +68,7 @@ /* static */ bool ScriptCompany::SetPresidentName(Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceCompanyModeValid(false); EnforcePrecondition(false, name != nullptr); diff --git a/src/script/api/script_goal.cpp b/src/script/api/script_goal.cpp index 416a8df295..cb7235f395 100644 --- a/src/script/api/script_goal.cpp +++ b/src/script/api/script_goal.cpp @@ -44,7 +44,7 @@ /* static */ ScriptGoal::GoalID ScriptGoal::New(ScriptCompany::CompanyID company, Text *goal, GoalType type, SQInteger destination) { - CCountedPtr counter(goal); + ScriptObjectRef counter(goal); EnforceDeityMode(GOAL_INVALID); EnforcePrecondition(GOAL_INVALID, goal != nullptr); @@ -79,7 +79,7 @@ /* static */ bool ScriptGoal::SetText(GoalID goal_id, Text *goal) { - CCountedPtr counter(goal); + ScriptObjectRef counter(goal); EnforcePrecondition(false, IsValidGoal(goal_id)); EnforceDeityMode(false); @@ -92,7 +92,7 @@ /* static */ bool ScriptGoal::SetProgress(GoalID goal_id, Text *progress) { - CCountedPtr counter(progress); + ScriptObjectRef counter(progress); EnforcePrecondition(false, IsValidGoal(goal_id)); EnforceDeityMode(false); @@ -119,7 +119,7 @@ /* static */ bool ScriptGoal::DoQuestion(SQInteger uniqueid, uint32_t target, bool is_client, Text *question, QuestionType type, SQInteger buttons) { - CCountedPtr counter(question); + ScriptObjectRef counter(question); EnforceDeityMode(false); EnforcePrecondition(false, question != nullptr); diff --git a/src/script/api/script_group.cpp b/src/script/api/script_group.cpp index 427aa48cb5..3f36fb6fca 100644 --- a/src/script/api/script_group.cpp +++ b/src/script/api/script_group.cpp @@ -56,7 +56,7 @@ /* static */ bool ScriptGroup::SetName(GroupID group_id, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceCompanyModeValid(false); EnforcePrecondition(false, IsValidGroup(group_id)); diff --git a/src/script/api/script_industry.cpp b/src/script/api/script_industry.cpp index 79e89a075f..d4db67dabd 100644 --- a/src/script/api/script_industry.cpp +++ b/src/script/api/script_industry.cpp @@ -58,7 +58,7 @@ /* static */ bool ScriptIndustry::SetText(IndustryID industry_id, Text *text) { - CCountedPtr counter(text); + ScriptObjectRef counter(text); EnforceDeityMode(false); EnforcePrecondition(false, IsValidIndustry(industry_id)); @@ -305,7 +305,7 @@ /* static */ bool ScriptIndustry::SetProductionLevel(IndustryID industry_id, SQInteger prod_level, bool show_news, Text *custom_news) { - CCountedPtr counter(custom_news); + ScriptObjectRef counter(custom_news); EnforceDeityMode(false); EnforcePrecondition(false, IsValidIndustry(industry_id)); diff --git a/src/script/api/script_league.cpp b/src/script/api/script_league.cpp index a2b6b93795..cc8fbec712 100644 --- a/src/script/api/script_league.cpp +++ b/src/script/api/script_league.cpp @@ -26,9 +26,9 @@ /* static */ ScriptLeagueTable::LeagueTableID ScriptLeagueTable::New(Text *title, Text *header, Text *footer) { - CCountedPtr title_counter(title); - CCountedPtr header_counter(header); - CCountedPtr footer_counter(footer); + ScriptObjectRef title_counter(title); + ScriptObjectRef header_counter(header); + ScriptObjectRef footer_counter(footer); EnforceDeityMode(LEAGUE_TABLE_INVALID); EnforcePrecondition(LEAGUE_TABLE_INVALID, title != nullptr); @@ -51,8 +51,8 @@ /* static */ ScriptLeagueTable::LeagueTableElementID ScriptLeagueTable::NewElement(ScriptLeagueTable::LeagueTableID table, SQInteger rating, ScriptCompany::CompanyID company, Text *text, Text *score, LinkType link_type, SQInteger link_target) { - CCountedPtr text_counter(text); - CCountedPtr score_counter(score); + ScriptObjectRef text_counter(text); + ScriptObjectRef score_counter(score); EnforceDeityMode(LEAGUE_TABLE_ELEMENT_INVALID); @@ -80,7 +80,7 @@ /* static */ bool ScriptLeagueTable::UpdateElementData(LeagueTableElementID element, ScriptCompany::CompanyID company, Text *text, LinkType link_type, SQInteger link_target) { - CCountedPtr text_counter(text); + ScriptObjectRef text_counter(text); EnforceDeityMode(false); EnforcePrecondition(false, IsValidLeagueTableElement(element)); @@ -100,7 +100,7 @@ /* static */ bool ScriptLeagueTable::UpdateElementScore(LeagueTableElementID element, SQInteger rating, Text *score) { - CCountedPtr score_counter(score); + ScriptObjectRef score_counter(score); EnforceDeityMode(false); EnforcePrecondition(false, IsValidLeagueTableElement(element)); diff --git a/src/script/api/script_news.cpp b/src/script/api/script_news.cpp index 93462436f0..f195a754e1 100644 --- a/src/script/api/script_news.cpp +++ b/src/script/api/script_news.cpp @@ -22,7 +22,7 @@ /* static */ bool ScriptNews::Create(NewsType type, Text *text, ScriptCompany::CompanyID company, NewsReferenceType ref_type, SQInteger reference) { - CCountedPtr counter(text); + ScriptObjectRef counter(text); EnforceDeityMode(false); EnforcePrecondition(false, text != nullptr); diff --git a/src/script/api/script_object.cpp b/src/script/api/script_object.cpp index 637d662125..cc94b18ff0 100644 --- a/src/script/api/script_object.cpp +++ b/src/script/api/script_object.cpp @@ -25,6 +25,21 @@ #include "../../safeguards.h" +void SimpleCountedObject::Release() +{ + int32_t res = --this->ref_count; + assert(res >= 0); + if (res == 0) { + try { + this->FinalRelease(); // may throw, for example ScriptTest/ExecMode + } catch (...) { + delete this; + throw; + } + delete this; + } +} + /** * Get the storage associated with the current ScriptInstance. * @return The storage. diff --git a/src/script/api/script_object.hpp b/src/script/api/script_object.hpp index 28cac81d59..10ad2aefd9 100644 --- a/src/script/api/script_object.hpp +++ b/src/script/api/script_object.hpp @@ -10,7 +10,6 @@ #ifndef SCRIPT_OBJECT_HPP #define SCRIPT_OBJECT_HPP -#include "../../misc/countedptr.hpp" #include "../../road_type.h" #include "../../rail_type.h" #include "../../string_func.h" @@ -34,6 +33,27 @@ typedef bool (ScriptModeProc)(); */ typedef bool (ScriptAsyncModeProc)(); +/** + * Simple counted object. Use it as base of your struct/class if you want to use + * basic reference counting. Your struct/class will destroy and free itself when + * last reference to it is released (using Release() method). The initial reference + * count (when it is created) is zero (don't forget AddRef() at least one time if + * not using ScriptObjectRef. + * @api -all + */ +class SimpleCountedObject { +public: + SimpleCountedObject() : ref_count(0) {} + virtual ~SimpleCountedObject() {} + + inline void AddRef() { ++this->ref_count; } + void Release(); + virtual void FinalRelease() {}; + +private: + int32_t ref_count; +}; + /** * Uper-parent object of all API classes. You should never use this class in * your script, as it doesn't publish any public functions. It is used @@ -406,7 +426,7 @@ public: */ ScriptObjectRef(T *data) : data(data) { - this->data->AddRef(); + if (this->data != nullptr) this->data->AddRef(); } /* No copy constructor. */ diff --git a/src/script/api/script_sign.cpp b/src/script/api/script_sign.cpp index 59077aa55c..19fcd04abb 100644 --- a/src/script/api/script_sign.cpp +++ b/src/script/api/script_sign.cpp @@ -35,7 +35,7 @@ /* static */ bool ScriptSign::SetName(SignID sign_id, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceDeityOrCompanyModeValid(false); EnforcePrecondition(false, IsValidSign(sign_id)); @@ -72,7 +72,7 @@ /* static */ SignID ScriptSign::BuildSign(TileIndex location, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceDeityOrCompanyModeValid(INVALID_SIGN); EnforcePrecondition(INVALID_SIGN, ::IsValidTile(location)); diff --git a/src/script/api/script_story_page.cpp b/src/script/api/script_story_page.cpp index 0476c16ef1..d1cd5004b0 100644 --- a/src/script/api/script_story_page.cpp +++ b/src/script/api/script_story_page.cpp @@ -45,7 +45,7 @@ static inline bool StoryPageElementTypeRequiresText(StoryPageElementType type) /* static */ ScriptStoryPage::StoryPageID ScriptStoryPage::New(ScriptCompany::CompanyID company, Text *title) { - CCountedPtr counter(title); + ScriptObjectRef counter(title); EnforceDeityMode(STORY_PAGE_INVALID); EnforcePrecondition(STORY_PAGE_INVALID, company == ScriptCompany::COMPANY_INVALID || ScriptCompany::ResolveCompanyID(company) != ScriptCompany::COMPANY_INVALID); @@ -62,7 +62,7 @@ static inline bool StoryPageElementTypeRequiresText(StoryPageElementType type) /* static */ ScriptStoryPage::StoryPageElementID ScriptStoryPage::NewElement(StoryPageID story_page_id, StoryPageElementType type, SQInteger reference, Text *text) { - CCountedPtr counter(text); + ScriptObjectRef counter(text); ::StoryPageElementType btype = static_cast<::StoryPageElementType>(type); @@ -109,7 +109,7 @@ static inline bool StoryPageElementTypeRequiresText(StoryPageElementType type) /* static */ bool ScriptStoryPage::UpdateElement(StoryPageElementID story_page_element_id, SQInteger reference, Text *text) { - CCountedPtr counter(text); + ScriptObjectRef counter(text); EnforceDeityMode(false); EnforcePrecondition(false, IsValidStoryPageElement(story_page_element_id)); @@ -165,7 +165,7 @@ static inline bool StoryPageElementTypeRequiresText(StoryPageElementType type) /* static */ bool ScriptStoryPage::SetTitle(StoryPageID story_page_id, Text *title) { - CCountedPtr counter(title); + ScriptObjectRef counter(title); EnforcePrecondition(false, IsValidStoryPage(story_page_id)); EnforceDeityMode(false); diff --git a/src/script/api/script_town.cpp b/src/script/api/script_town.cpp index 3a4e302667..7c601e3884 100644 --- a/src/script/api/script_town.cpp +++ b/src/script/api/script_town.cpp @@ -42,7 +42,7 @@ /* static */ bool ScriptTown::SetName(TownID town_id, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceDeityMode(false); EnforcePrecondition(false, IsValidTown(town_id)); @@ -57,7 +57,7 @@ /* static */ bool ScriptTown::SetText(TownID town_id, Text *text) { - CCountedPtr counter(text); + ScriptObjectRef counter(text); EnforceDeityMode(false); EnforcePrecondition(false, IsValidTown(town_id)); @@ -283,7 +283,7 @@ /* static */ bool ScriptTown::FoundTown(TileIndex tile, TownSize size, bool city, RoadLayout layout, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceDeityOrCompanyModeValid(false); EnforcePrecondition(false, ScriptCompanyMode::IsDeity() || _settings_game.economy.found_town != TF_FORBIDDEN); diff --git a/src/script/api/script_vehicle.cpp b/src/script/api/script_vehicle.cpp index 35a6d74dda..c73ddee05c 100644 --- a/src/script/api/script_vehicle.cpp +++ b/src/script/api/script_vehicle.cpp @@ -247,7 +247,7 @@ /* static */ bool ScriptVehicle::SetName(VehicleID vehicle_id, Text *name) { - CCountedPtr counter(name); + ScriptObjectRef counter(name); EnforceCompanyModeValid(false); EnforcePrecondition(false, IsPrimaryVehicle(vehicle_id)); diff --git a/src/script/script_info.hpp b/src/script/script_info.hpp index 90a04d13d8..9c01bca671 100644 --- a/src/script/script_info.hpp +++ b/src/script/script_info.hpp @@ -11,8 +11,8 @@ #define SCRIPT_INFO_HPP #include -#include "../misc/countedptr.hpp" +#include "script_object.hpp" #include "script_config.hpp" /** The maximum number of operations for saving or loading the data of a script. */