From 170f2ee554ada5f274d24579cf23ab98323df066 Mon Sep 17 00:00:00 2001
From: Giovanni Bussi <giovanni.bussi@gmail.com>
Date: Wed, 6 Sep 2017 16:26:12 +0200
Subject: [PATCH] unique_ptr in ActionSet

This change required some small fixes here and there.

In particular, the code accessing elements of ActionSet thinking it was
a pointer now need a .get() in order to access the pointer (e.g. for
a dynamic_cast)
---
 src/bias/MetaD.cpp           | 2 +-
 src/core/ActionAtomistic.cpp | 2 +-
 src/core/ActionSet.cpp       | 9 +++++----
 src/core/ActionSet.h         | 9 +++++----
 src/core/ActionSetup.cpp     | 2 +-
 src/core/PlumedMain.cpp      | 9 +++++----
 src/generic/Debug.cpp        | 4 ++--
 7 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/bias/MetaD.cpp b/src/bias/MetaD.cpp
index 3b458f403..393452b6c 100644
--- a/src/bias/MetaD.cpp
+++ b/src/bias/MetaD.cpp
@@ -1160,7 +1160,7 @@ MetaD::MetaD(const ActionOptions& ao):
 
   bool concurrent=false;
   const ActionSet&actionSet(plumed.getActionSet());
-  for(const auto & p : actionSet) if(dynamic_cast<MetaD*>(p)) { concurrent=true; break; }
+  for(const auto & p : actionSet) if(dynamic_cast<MetaD*>(p.get())) { concurrent=true; break; }
   if(concurrent) log<<"  You are using concurrent metadynamics\n";
   if(rect_biasf_.size()>0) {
     if(walkers_mpi) {
diff --git a/src/core/ActionAtomistic.cpp b/src/core/ActionAtomistic.cpp
index 8973b6866..35f6086d2 100644
--- a/src/core/ActionAtomistic.cpp
+++ b/src/core/ActionAtomistic.cpp
@@ -197,7 +197,7 @@ void ActionAtomistic::interpretAtomList( std::vector<std::string>& strings, std:
     if(!ok) {
       const ActionSet&actionSet(plumed.getActionSet());
       for(const auto & a : actionSet) {
-        ActionWithVirtualAtom* c=dynamic_cast<ActionWithVirtualAtom*>(a);
+        ActionWithVirtualAtom* c=dynamic_cast<ActionWithVirtualAtom*>(a.get());
         if(c) if(c->getLabel()==strings[i]) {
             ok=true;
             t.push_back(c->getIndex());
diff --git a/src/core/ActionSet.cpp b/src/core/ActionSet.cpp
index 35334eca2..cbc9955e0 100644
--- a/src/core/ActionSet.cpp
+++ b/src/core/ActionSet.cpp
@@ -31,11 +31,12 @@ ActionSet::ActionSet(PlumedMain&p):
 
 ActionSet::~ActionSet()
 {
-  for(int i=size()-1; i>=0; i--) delete (*this)[i];
+// required in order to deallocate in reverse order:
+  for(int i=size()-1; i>=0; i--) (*this)[i].reset();
 }
 
 void ActionSet::clearDelete() {
-  for(int i=size()-1; i>=0; i--) delete (*this)[i];
+  for(int i=size()-1; i>=0; i--) (*this)[i].reset();
   clear();
 }
 
@@ -43,7 +44,7 @@ void ActionSet::clearDelete() {
 std::string ActionSet::getLabelList() const {
   std::string outlist;
   for(const auto & p : (*this)) {
-    outlist+=dynamic_cast<Action*>(p)->getLabel()+" ";
+    outlist+=dynamic_cast<Action*>(p.get())->getLabel()+" ";
   };
   return  outlist;
 }
@@ -51,7 +52,7 @@ std::string ActionSet::getLabelList() const {
 std::vector<std::string> ActionSet::getLabelVector() const {
   std::vector<std::string> outlist;
   for(const auto & p : (*this)) {
-    outlist.push_back(dynamic_cast<Action*>(p)->getLabel());
+    outlist.push_back(dynamic_cast<Action*>(p.get())->getLabel());
   };
   return  outlist;
 }
diff --git a/src/core/ActionSet.h b/src/core/ActionSet.h
index c2d48a269..24a12dc2b 100644
--- a/src/core/ActionSet.h
+++ b/src/core/ActionSet.h
@@ -23,6 +23,7 @@
 #define __PLUMED_core_ActionSet_h
 
 #include "Action.h"
+#include <memory>
 
 namespace PLMD {
 
@@ -36,7 +37,7 @@ class PlumedMain;
 /// Finally, since it holds pointers, there is a clearDelete() function
 /// which deletes the pointers before deleting the vector
 class ActionSet:
-  public std::vector<Action*>
+  public std::vector<std::unique_ptr<Action>>
 {
   PlumedMain& plumed;
 public:
@@ -73,7 +74,7 @@ template <class T>
 std::vector<T> ActionSet::select()const {
   std::vector<T> ret;
   for(const auto & p : (*this)) {
-    T t=dynamic_cast<T>(p);
+    T t=dynamic_cast<T>(p.get());
     if(t) ret.push_back(t);
   };
   return ret;
@@ -82,7 +83,7 @@ std::vector<T> ActionSet::select()const {
 template <class T>
 T ActionSet::selectWithLabel(const std::string&s)const {
   for(const auto & p : (*this)) {
-    T t=dynamic_cast<T>(p);
+    T t=dynamic_cast<T>(p.get());
     if(t && dynamic_cast<Action*>(t)->getLabel()==s) return t;
   };
   return NULL;
@@ -93,7 +94,7 @@ std::vector<Action*> ActionSet::selectNot()const {
   std::vector<Action*> ret;
   for(const auto & p : (*this)) {
     T t=dynamic_cast<T>(p);
-    if(!t) ret.push_back(p);
+    if(!t) ret.push_back(p.get());
   };
   return ret;
 }
diff --git a/src/core/ActionSetup.cpp b/src/core/ActionSetup.cpp
index 1a0216a7b..b9db51e3b 100644
--- a/src/core/ActionSetup.cpp
+++ b/src/core/ActionSetup.cpp
@@ -32,7 +32,7 @@ ActionSetup::ActionSetup(const ActionOptions&ao):
   const ActionSet& actionset(plumed.getActionSet());
   for(const auto & p : actionset) {
 // check that all the preceeding actions are ActionSetup
-    if( !dynamic_cast<ActionSetup*>(p) ) error("Action " + getLabel() + " is a setup action, and should be only preceeded by other setup actions");
+    if( !dynamic_cast<ActionSetup*>(p.get()) ) error("Action " + getLabel() + " is a setup action, and should be only preceeded by other setup actions");
   }
 }
 
diff --git a/src/core/PlumedMain.cpp b/src/core/PlumedMain.cpp
index 761062b8c..9a28cc0a0 100644
--- a/src/core/PlumedMain.cpp
+++ b/src/core/PlumedMain.cpp
@@ -530,7 +530,7 @@ void PlumedMain::readInputWords(const std::vector<std::string> & words) {
   } else {
     std::vector<std::string> interpreted(words);
     Tools::interpretLabel(interpreted);
-    Action* action=actionRegister().create(ActionOptions(*this,interpreted));
+    std::unique_ptr<Action> action(actionRegister().create(ActionOptions(*this,interpreted)));
     if(!action) {
       log<<"ERROR\n";
       log<<"I cannot understand line:";
@@ -540,7 +540,7 @@ void PlumedMain::readInputWords(const std::vector<std::string> & words) {
       plumed_merror("I cannot understand line " + interpreted[0] + " " + interpreted[1]);
     };
     action->checkRead();
-    actionSet.push_back(action);
+    actionSet.emplace_back(std::move(action));
   };
 
   pilots=actionSet.select<ActionPilot*>();
@@ -640,7 +640,8 @@ void PlumedMain::justCalculate() {
 
   int iaction=0;
 // calculate the active actions in order (assuming *backward* dependence)
-  for(const auto & p : actionSet) {
+  for(const auto & pp : actionSet) {
+    Action* p(pp.get());
     if(p->isActive()) {
       std::string actionNumberLabel;
       if(detailedTimers) {
@@ -684,7 +685,7 @@ void PlumedMain::backwardPropagate() {
   stopwatch.start("5 Applying (backward loop)");
 // apply them in reverse order
   for(auto pp=actionSet.rbegin(); pp!=actionSet.rend(); ++pp) {
-    const auto & p(*pp);
+    const auto & p(pp->get());
     if(p->isActive()) {
 
       std::string actionNumberLabel;
diff --git a/src/generic/Debug.cpp b/src/generic/Debug.cpp
index 36654b0d9..b2d10ae96 100644
--- a/src/generic/Debug.cpp
+++ b/src/generic/Debug.cpp
@@ -109,13 +109,13 @@ void Debug::apply() {
     const ActionSet&actionSet(plumed.getActionSet());
     int a=0;
     for(const auto & p : actionSet) {
-      if(dynamic_cast<Debug*>(p))continue;
+      if(dynamic_cast<Debug*>(p.get()))continue;
       if(p->isActive()) a++;
     };
     if(a>0) {
       ofile.printf("activity at step %i: ",getStep());
       for(const auto & p : actionSet) {
-        if(dynamic_cast<Debug*>(p))continue;
+        if(dynamic_cast<Debug*>(p.get()))continue;
         if(p->isActive()) ofile.printf("+");
         else                 ofile.printf("-");
       };
-- 
GitLab