Convert bespoke game element reference counting to use std::shared_ptr.

For historical reasons Gambit has had its own `GameObject` base class, which
combined tracking of object validity (i.e. not having been deleted from
its game) with reference-counting (for references to the object
in calling code).

This modernises this mechanism and more clearly separates the two concerns:
* Reference-counting is now done by using `std::shared_ptr`.  Game objects are 'owned'
  by the collections to which they belong, in terms
  of whether the game itself contains a reference to an allocated object.
* Rather than having a separate base class which provides a validity flag (as that would be
  all that remains in such a class), `GameObject` has been removed
  and instead the `GameObjectPtr` template simply assumes that game elements provide an `IsValid` member.

Closes #518.

Author: tturocy

.github/workflows/tools.yml

@@ -40,11 +40,11 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Install dependencies
-        run: brew install automake autoconf wxwidgets
+        run: brew install automake autoconf wxwidgets@3.2
       - run: aclocal
       - run: automake --add-missing
       - run: autoconf
-      - run: ./configure
+      - run: ./configure --with-wx-config=wx-config-3.2
       - run: make
       - run: sudo make install
       - run: make osx-dmg

ChangeLog

@@ -19,6 +19,8 @@
 - Implement `Nodes` collection as a member of `GameRep`, including a C++ iterator that
   returns nodes in depth-first traversal order (#530)
 
+### Changed
+- Internally in C++ `std::shared_ptr` are now used to manage memory allocated for game objects.  (#518)
 
 ## [16.3.1] - unreleased
 

src/games/behavmixed.cc

@@ -98,7 +98,7 @@ void MixedBehaviorProfile<T>::RealizationProbs(const MixedStrategyProfile<T> &mp
     if (node->GetPlayer() && !node->GetPlayer()->IsChance()) {
       if (node->GetPlayer() == player) {
         if (contains(actions, node->m_infoset) &&
-            actions.at(node->GetInfoset()) == static_cast<int>(i)) {
+            actions.at(node->GetInfoset().get()) == static_cast<int>(i)) {
           prob = T(1);
         }
         else {
@@ -117,12 +117,12 @@ void MixedBehaviorProfile<T>::RealizationProbs(const MixedStrategyProfile<T> &mp
       prob = T(node->m_infoset->GetActionProb(node->m_infoset->GetAction(i)));
     }
 
-    GameNodeRep *child = node->m_children[i - 1];
+    auto child = node->m_children[i - 1];
 
-    map_bvals[child] = prob * map_bvals[node];
+    map_bvals[child] = prob * map_bvals[node->shared_from_this()];
     map_nvals[child] += map_bvals[child];
 
-    RealizationProbs(mp, player, actions, child, map_nvals, map_bvals);
+    RealizationProbs(mp, player, actions, child.get(), map_nvals, map_bvals);
   }
 }
 
@@ -141,7 +141,7 @@ MixedBehaviorProfile<T>::MixedBehaviorProfile(const MixedStrategyProfile<T> &p_p
     }
   }
 
-  GameNodeRep *root = m_support.GetGame()->GetRoot();
+  GameNodeRep *root = m_support.GetGame()->GetRoot().get();
 
   const StrategySupportProfile &support = p_profile.GetSupport();
   GameRep *game = m_support.GetGame().get();
@@ -151,11 +151,11 @@ MixedBehaviorProfile<T>::MixedBehaviorProfile(const MixedStrategyProfile<T> &p_p
     for (auto strategy : support.GetStrategies(player)) {
       if (p_profile[strategy] > T(0)) {
         const auto &actions = strategy->m_behav;
-        map_bvals[root] = p_profile[strategy];
+        map_bvals[root->shared_from_this()] = p_profile[strategy];
         RealizationProbs(p_profile, player, actions, root, map_nvals, map_bvals);
       }
     }
-    map_nvals[root] = T(1); // set the root nval
+    map_nvals[root->shared_from_this()] = T(1); // set the root nval
     auto root = m_support.GetGame()->GetRoot();
     BehaviorStrat(player, root, map_nvals, map_bvals);
   }

src/games/game.cc

@@ -42,7 +42,7 @@ namespace Gambit {
 GameOutcomeRep::GameOutcomeRep(GameRep *p_game, int p_number) : m_game(p_game), m_number(p_number)
 {
   for (const auto &player : m_game->m_players) {
-    m_payoffs[player] = Number();
+    m_payoffs[player.get()] = Number();
   }
 }
 
@@ -52,11 +52,11 @@ GameOutcomeRep::GameOutcomeRep(GameRep *p_game, int p_number) : m_game(p_game),
 
 GameAction GameStrategyRep::GetAction(const GameInfoset &p_infoset) const
 {
-  if (p_infoset->GetPlayer() != m_player) {
+  if (p_infoset->GetPlayer().get() != m_player) {
     throw MismatchException();
   }
   try {
-    return *std::next(p_infoset->GetActions().cbegin(), m_behav.at(p_infoset) - 1);
+    return *std::next(p_infoset->GetActions().cbegin(), m_behav.at(p_infoset.get()) - 1);
   }
   catch (std::out_of_range &) {
     return nullptr;
@@ -71,7 +71,7 @@ GamePlayerRep::GamePlayerRep(GameRep *p_game, int p_id, int p_strats)
   : m_game(p_game), m_number(p_id)
 {
   for (int j = 1; j <= p_strats; j++) {
-    m_strategies.push_back(new GameStrategyRep(this, j, ""));
+    m_strategies.push_back(std::make_shared<GameStrategyRep>(this, j, ""));
   }
 }
 
@@ -87,11 +87,12 @@ GamePlayerRep::~GamePlayerRep()
 
 void GamePlayerRep::MakeStrategy(const std::map<GameInfosetRep *, int> &behav)
 {
-  auto *strategy = new GameStrategyRep(this, m_strategies.size() + 1, "");
+  auto strategy = std::make_shared<GameStrategyRep>(this, m_strategies.size() + 1, "");
   strategy->m_behav = behav;
   for (const auto &infoset : m_infosets) {
-    strategy->m_label +=
-        (contains(strategy->m_behav, infoset)) ? std::to_string(strategy->m_behav[infoset]) : "*";
+    strategy->m_label += (contains(strategy->m_behav, infoset.get()))
+                             ? std::to_string(strategy->m_behav[infoset.get()])
+                             : "*";
   }
   if (strategy->m_label.empty()) {
     strategy->m_label = "*";
@@ -109,7 +110,7 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn,
       if (!contains(behav, n->m_infoset)) {
         // we haven't visited this infoset before
         for (size_t i = 1; i <= n->m_children.size(); i++) {
-          GameNodeRep *m = n->m_children[i - 1];
+          GameNodeRep *m = n->m_children[i - 1].get();
           whichbranch[n] = m;
           behav[n->m_infoset] = i;
           MakeReducedStrats(m, nn, behav, ptr, whichbranch);
@@ -118,7 +119,8 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn,
       }
       else {
         // we have visited this infoset, take same action
-        MakeReducedStrats(n->m_children[behav[n->m_infoset] - 1], nn, behav, ptr, whichbranch);
+        MakeReducedStrats(n->m_children[behav[n->m_infoset] - 1].get(), nn, behav, ptr,
+                          whichbranch);
       }
     }
     else {
@@ -128,12 +130,13 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn,
       else {
         ptr.erase(n);
       }
-      whichbranch[n] = n->m_children.front();
-      MakeReducedStrats(n->m_children.front(), n->m_children.front(), behav, ptr, whichbranch);
+      whichbranch[n] = n->m_children.front().get();
+      MakeReducedStrats(n->m_children.front().get(), n->m_children.front().get(), behav, ptr,
+                        whichbranch);
     }
   }
   else if (nn) {
-    GameNodeRep *m;
+    GameNode m;
     for (;; nn = whichbranch.at(ptr.at(nn->m_parent))) {
       m = nn->GetNextSibling();
       if (m || !contains(ptr, nn->m_parent)) {
@@ -142,8 +145,8 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn,
     }
     if (m) {
       GameNodeRep *mm = whichbranch.at(m->m_parent);
-      whichbranch[m->m_parent] = m;
-      MakeReducedStrats(m, m, behav, ptr, whichbranch);
+      whichbranch[m->m_parent] = m.get();
+      MakeReducedStrats(m.get(), m.get(), behav, ptr, whichbranch);
       whichbranch[m->m_parent] = mm;
     }
     else {
@@ -160,8 +163,9 @@ size_t GamePlayerRep::NumSequences() const
   if (!m_game->IsTree()) {
     throw UndefinedException();
   }
-  return std::transform_reduce(m_infosets.cbegin(), m_infosets.cend(), 1, std::plus<>(),
-                               [](const GameInfosetRep *s) { return s->m_actions.size(); });
+  return std::transform_reduce(
+      m_infosets.cbegin(), m_infosets.cend(), 1, std::plus<>(),
+      [](const std::shared_ptr<GameInfosetRep> &s) { return s->m_actions.size(); });
 }
 
 //========================================================================
@@ -318,8 +322,8 @@ MixedStrategyProfile<T>::MixedStrategyProfile(const MixedBehaviorProfile<T> &p_p
     for (const auto &strategy : player->m_strategies) {
       auto prob = static_cast<T>(1);
       for (const auto &infoset : player->m_infosets) {
-        if (strategy->m_behav[infoset] > 0) {
-          prob *= p_profile[infoset->GetAction(strategy->m_behav[infoset])];
+        if (strategy->m_behav[infoset.get()] > 0) {
+          prob *= p_profile[infoset->GetAction(strategy->m_behav[infoset.get()])];
         }
       }
       (*m_rep)[strategy] = prob;
@@ -342,7 +346,7 @@ MixedStrategyProfile<T>::operator=(const MixedStrategyProfile<T> &p_profile)
 //             MixedStrategyProfile<T>: General data access
 //========================================================================
 
-template <class T> Vector<T> MixedStrategyProfile<T>::operator[](const GamePlayer &p_player) const
+template <class T> Vector<T> MixedStrategyProfile<T>::GetStrategy(const GamePlayer &p_player) const
 {
   CheckVersion();
   auto strategies = m_rep->m_support.GetStrategies(p_player);