Use smart pointers for internal memory management.
authorRoland Schulz <roland@utk.edu>
Sat, 3 Mar 2012 00:16:07 +0000 (19:16 -0500)
committerTeemu Murtola <teemu.murtola@gmail.com>
Sat, 17 Mar 2012 14:28:12 +0000 (16:28 +0200)
- Replaced uses of std::auto_ptr with boost::scoped_ptr where memory was
  allocated locally and release() was not used, or refactored the use of
  pointers out completely.
- Use gmx_unique_ptr in containers that are responsible for freeing
  the contained objects.
- Removed an unnecessary #include <memory>.
- Also added doxygen comments in modified parts to satisfy Jenkins.

Part of issue #887.

Change-Id: Ib582b6395fb348d392a742acd4b1ff73acfa74a3

25 files changed:
CMakeLists.txt
cmake/gmxCFlags.cmake
cmake/gmxTestCXX11.cmake [new file with mode: 0644]
share/template/template.cpp
src/config.h.cmakein
src/gromacs/analysisdata/abstractdata-impl.h
src/gromacs/analysisdata/abstractdata.cpp
src/gromacs/analysisdata/analysisdata-impl.h
src/gromacs/analysisdata/analysisdata.cpp
src/gromacs/analysisdata/datastorage-impl.h
src/gromacs/analysisdata/datastorage.cpp
src/gromacs/analysisdata/datastorage.h
src/gromacs/analysisdata/tests/arraydata.cpp
src/gromacs/analysisdata/tests/mock_module-impl.h
src/gromacs/analysisdata/tests/mock_module.cpp
src/gromacs/options/options-impl.h
src/gromacs/options/options.cpp
src/gromacs/options/optionstoragetemplate.h
src/gromacs/selection/compiler.cpp
src/gromacs/selection/evaluate.cpp
src/gromacs/selection/parsetree.cpp
src/gromacs/selection/selectioncollection-impl.h
src/gromacs/selection/selectioncollection.cpp
src/gromacs/utility/CMakeLists.txt
src/gromacs/utility/uniqueptr.h [new file with mode: 0644]

index 9e328b3ef386ad5cabdf79a3c4e9f41d23709db8..13f1ceecc576df9f8b8db5a7649a7969e8c6cffe 100644 (file)
@@ -544,6 +544,10 @@ include(gmxTestInlineASM)
 gmx_test_inline_asm_gcc_x86(GMX_X86_GCC_INLINE_ASM)
 gmx_test_inline_asm_msvc_x86(GMX_X86_MSVC_INLINE_ASM)
 
+include(gmxTestCXX11)
+gmx_test_cxx11(HAVE_CXX11 CXX11_FLAG)
+set(GROMACS_CXX_FLAGS "${CXX11_FLAG} ${GROMACS_CXX_FLAGS}")
+
 # turn on SSE if supported with reasonable defaults.
 if (${GMX_ACCELERATION} STREQUAL "auto" AND NOT GMX_OPENMM)
   if(CMAKE_SYSTEM_PROCESSOR MATCHES "^(i.86|x86|x64|x86_64|AMD64|amd64)" OR CYGWIN)
index 6739ee0d872f6282c158330c857ec070663c69e9..edcab5819458549f772b3c340a0b12aa3eaa30de 100644 (file)
@@ -74,7 +74,6 @@ MACRO(gmx_c_flags)
 
     if (CMAKE_CXX_COMPILER_ID MATCHES "Intel")
         if (NOT WIN32) 
-            GMX_TEST_CXXFLAG(CXXFLAGS_OPT "-std=gnu99" GMXC_CXXFLAGS)
             GMX_TEST_CXXFLAG(CXXFLAGS_OPT "-ip -funroll-all-loops" GMXC_CXXFLAGS_RELEASE)
             GMX_TEST_CXXFLAG(CXXFLAGS_SSE2 "-msse2" GMXC_CXXFLAGS_RELEASE)
             GMX_TEST_CXXFLAG(CXXFLAGS_X86 "-mtune=core2" GMXC_CXXFLAGS_RELEASE)
diff --git a/cmake/gmxTestCXX11.cmake b/cmake/gmxTestCXX11.cmake
new file mode 100644 (file)
index 0000000..101ad4a
--- /dev/null
@@ -0,0 +1,38 @@
+include(CheckCXXSourceCompiles)
+MACRO(GMX_TEST_CXX11 VARIABLE FLAG)
+  MESSAGE(STATUS "Checking for C++11 support")
+  if(NOT WIN32)
+    set(CXX11_FLAG "-std=c++0x")
+  else()
+    set(CXX11_FLAG "/Qstd=c++0x")
+  endif()
+  CHECK_CXX_COMPILER_FLAG("${CXX11_FLAG}" CXXFLAG_STD_CXX0X)
+  if(NOT CXXFLAG_STD_CXX0X)
+    set(CXX11_FLAG "")
+  endif()
+  set(CMAKE_REQUIRED_DEFINITIONS "${CXX11_FLAG}")
+  check_cxx_source_compiles(
+"#include <vector>
+#include <memory>
+#include <utility>
+struct A {
+  std::unique_ptr<int> p;
+};
+int main() {
+  typedef std::unique_ptr<int> intPointer;
+  intPointer p(new int(10));
+  std::vector<intPointer> v;
+  v.push_back(std::move(p));
+  std::vector<A> v2;
+  v2.push_back(A());  //requires default move constructor
+}" CXX11_SUPPORT_OK)
+  set(CMAKE_REQUIRED_DEFINITIONS "")
+  if(CXX11_SUPPORT_OK)
+    set(${VARIABLE} ${CXX11_SUPPORT_OK})
+    set(${FLAG} ${CXX11_FLAG})
+    MESSAGE(STATUS "Checking for C++11 support - yes")
+  else()
+    set(${FLAG} "")
+    MESSAGE(STATUS "Checking for C++11 support - no")
+  endif()
+ENDMACRO()
\ No newline at end of file
index cb54d9e4a8cd6b30cc2d87dc0e1ec13e85584ba2..1daa79c44c785939065569301b8f8d33cb78e970 100644 (file)
@@ -28,7 +28,6 @@
  *
  * For more info, check our website at http://www.gromacs.org
  */
-#include <memory>
 #include <string>
 #include <vector>
 
@@ -75,25 +74,32 @@ class AnalysisTemplate : public TrajectoryAnalysisModule
 class AnalysisTemplate::ModuleData : public TrajectoryAnalysisModuleData
 {
     public:
+        /*! \brief
+         * Initializes frame-local data.
+         *
+         * \param[in] module     Analysis module to use for data objects.
+         * \param[in] opt        Data parallelization options.
+         * \param[in] selections Thread-local selection collection.
+         * \param[in] cutoff     Cutoff distance for the search
+         *   (<=0 stands for no cutoff).
+         * \param[in] posCount   Maximum number of reference particles.
+         */
         ModuleData(TrajectoryAnalysisModule *module,
                    const AnalysisDataParallelOptions &opt,
-                   const SelectionCollection &selections)
+                   const SelectionCollection &selections,
+                   double cutoff, int posCount)
             : TrajectoryAnalysisModuleData(module, opt, selections),
-              _nb(NULL)
+              _nb(cutoff, posCount)
         {
         }
 
-        virtual ~ModuleData()
-        {
-            delete _nb;
-        }
-
         virtual void finish()
         {
             finishDataHandles();
         }
 
-        NeighborhoodSearch          *_nb;
+        //! Neighborhood search data for distance calculation.
+        NeighborhoodSearch      _nb;
 };
 
 
@@ -119,7 +125,7 @@ AnalysisTemplate::initOptions(TrajectoryAnalysisSettings *settings)
         "To get started with implementing your own analysis program,",
         "follow the instructions in the README file provided.",
         "This template implements a simple analysis programs that calculates",
-        "average distances from the a reference group to one or more",
+        "average distances from a reference group to one or more",
         "analysis groups.",
         NULL
     };
@@ -174,9 +180,7 @@ TrajectoryAnalysisModuleData *
 AnalysisTemplate::startFrames(const AnalysisDataParallelOptions &opt,
                               const SelectionCollection &selections)
 {
-    std::auto_ptr<ModuleData> pdata(new ModuleData(this, opt, selections));
-    pdata->_nb = new NeighborhoodSearch(_cutoff, _refsel->posCount());
-    return pdata.release();
+    return new ModuleData(this, opt, selections, _cutoff, _refsel->posCount());
 }
 
 
@@ -185,9 +189,9 @@ AnalysisTemplate::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
                                TrajectoryAnalysisModuleData *pdata)
 {
     AnalysisDataHandle *dh = pdata->dataHandle("avedist");
-    NeighborhoodSearch *nb = static_cast<ModuleData *>(pdata)->_nb;
+    NeighborhoodSearch &nb = static_cast<ModuleData *>(pdata)->_nb;
 
-    nb->init(pbc, _refsel->positions());
+    nb.init(pbc, _refsel->positions());
     dh->startFrame(frnr, fr.time);
     for (size_t g = 0; g < _sel.size(); ++g)
     {
@@ -197,7 +201,7 @@ AnalysisTemplate::analyzeFrame(int frnr, const t_trxframe &fr, t_pbc *pbc,
         for (int i = 0; i < nr; ++i)
         {
             SelectionPosition p = sel->position(i);
-            frave += nb->minimumDistance(p.x());
+            frave += nb.minimumDistance(p.x());
         }
         frave /= nr;
         dh->setPoint(g, frave);
index 77ad877e8e8735eb0bca3d2d4aca31a01cac9fdf..3eaef8c443411b61e6d5eb1272dca0e667b6c009 100644 (file)
 /* Define if we have pipes */
 #cmakedefine HAVE_PIPES
 
+/* Define if we have sufficient C++11 support */
+#cmakedefine HAVE_CXX11
 
 /* Catch stupid CMake problems on OS X */
 #ifdef __APPLE__
index 5936af68dbedd8ab62cba42879c756d02af6a489..ef7737d60a9e543b86b9d71c8a41c1b0952d14a5 100644 (file)
@@ -40,7 +40,9 @@
 
 #include <vector>
 
-#include "types/simple.h"
+#include "../legacyheaders/types/simple.h"
+
+#include "gromacs/utility/uniqueptr.h"
 
 #include "abstractdata.h"
 #include "dataframe.h"
@@ -56,8 +58,10 @@ namespace gmx
 class AbstractAnalysisData::Impl
 {
     public:
+        //! Shorthand for a smart pointer to a module.
+        typedef gmx_unique_ptr<AnalysisDataModuleInterface>::type ModulePointer;
         //! Shorthand for list of modules added to the data.
-        typedef std::vector<AnalysisDataModuleInterface *> ModuleList;
+        typedef std::vector<ModulePointer> ModuleList;
 
         Impl();
         ~Impl();
index 5e69c27aa4c7d6a9dde3d4517ee87af8d6d951f6..19639587494c37b49a13d585a5748ce334467a6f 100644 (file)
@@ -61,11 +61,6 @@ AbstractAnalysisData::Impl::Impl()
 
 AbstractAnalysisData::Impl::~Impl()
 {
-    ModuleList::const_iterator i;
-    for (i = _modules.begin(); i != _modules.end(); ++i)
-    {
-        delete *i;
-    }
 }
 
 
@@ -156,7 +151,7 @@ AbstractAnalysisData::requestStorage(int nframes)
 void
 AbstractAnalysisData::addModule(AnalysisDataModuleInterface *module)
 {
-    std::auto_ptr<AnalysisDataModuleInterface> module_ptr(module);
+    Impl::ModulePointer module_ptr(module);
     if ((columnCount() > 1 && !(module->flags() & AnalysisDataModuleInterface::efAllowMulticolumn))
         || (isMultipoint() && !(module->flags() & AnalysisDataModuleInterface::efAllowMultipoint))
         || (!isMultipoint() && (module->flags() & AnalysisDataModuleInterface::efOnlyMultipoint)))
@@ -174,8 +169,7 @@ AbstractAnalysisData::addModule(AnalysisDataModuleInterface *module)
     {
         _impl->_bAllowMissing = false;
     }
-    _impl->_modules.push_back(module);
-    module_ptr.release();
+    _impl->_modules.push_back(move(module_ptr));
 }
 
 
index b8bbbbd01556092c5578bb85f2ab2cc245764eab..2b694e0118343feba94b04c0f0f0208797793ed1 100644 (file)
@@ -40,6 +40,8 @@
 
 #include <vector>
 
+#include "gromacs/utility/uniqueptr.h"
+
 #include "analysisdata.h"
 #include "datastorage.h"
 
@@ -54,8 +56,10 @@ namespace gmx
 class AnalysisData::Impl
 {
     public:
+        //! Shorthand for a smart pointer to a data handle.
+        typedef gmx_unique_ptr<AnalysisDataHandle>::type HandlePointer;
         //! Shorthand for a list of data handles.
-        typedef std::vector<AnalysisDataHandle *> HandleList;
+        typedef std::vector<HandlePointer> HandleList;
 
         Impl();
         ~Impl();
index c8d7b314accbab85b1ff697501bf905a4d950991..d1de779ab56451a2da625ade416aabc0a826681d 100644 (file)
@@ -37,9 +37,6 @@
  */
 #include "gromacs/analysisdata/analysisdata.h"
 
-#include <algorithm>
-#include <memory>
-
 #include "gromacs/analysisdata/dataframe.h"
 #include "gromacs/analysisdata/datastorage.h"
 #include "gromacs/analysisdata/paralleloptions.h"
@@ -62,11 +59,6 @@ AnalysisData::Impl::Impl()
 
 AnalysisData::Impl::~Impl()
 {
-    HandleList::const_iterator i;
-    for (i = handles_.begin(); i != handles_.end(); ++i)
-    {
-        delete *i;
-    }
 }
 
 
@@ -113,9 +105,9 @@ AnalysisData::startData(const AnalysisDataParallelOptions &opt)
         GMX_THROW(NotImplementedError("Parallelism not supported for multipoint data"));
     }
 
-    std::auto_ptr<AnalysisDataHandle> handle(new AnalysisDataHandle(this));
-    impl_->handles_.push_back(handle.get());
-    return handle.release();
+    Impl::HandlePointer handle(new AnalysisDataHandle(this));
+    impl_->handles_.push_back(move(handle));
+    return impl_->handles_.back().get();
 }
 
 
@@ -124,12 +116,17 @@ AnalysisData::finishData(AnalysisDataHandle *handle)
 {
     Impl::HandleList::iterator i;
 
-    i = std::find(impl_->handles_.begin(), impl_->handles_.end(), handle);
+    for (i = impl_->handles_.begin(); i != impl_->handles_.end(); ++i)
+    {
+        if (i->get() == handle)
+        {
+            break;
+        }
+    }
     GMX_RELEASE_ASSERT(i != impl_->handles_.end(),
                        "finishData() called for an unknown handle");
 
     impl_->handles_.erase(i);
-    delete handle;
 
     if (impl_->handles_.empty())
     {
index 4f12c985afe5f02a9ff8693662f61f612149f166..ec48148f03506eca34cfaaab44103a94e47a9fa9 100644 (file)
@@ -41,6 +41,8 @@
 #include <limits>
 #include <vector>
 
+#include "gromacs/utility/uniqueptr.h"
+
 #include "datastorage.h"
 
 namespace gmx
@@ -58,11 +60,15 @@ class AnalysisDataStorageFrame;
 class AnalysisDataStorage::Impl
 {
     public:
+        //! Smart pointer type for managing a stored frame.
+        typedef gmx_unique_ptr<AnalysisDataStorageFrame>::type FramePointer;
+
         /*! \brief
          * Stored information about a single stored frame.
          */
         struct StoredFrame
         {
+            //! Indicates what operations have been performed on a frame.
             enum Status
             {
                 eMissing,  //!< Frame has not yet been started.
@@ -71,24 +77,26 @@ class AnalysisDataStorage::Impl
                 eNotified  //!< Appropriate notifications have been sent.
             };
 
-            StoredFrame() : frame(NULL), status(eMissing) {}
+            //! Constructs an object that manages a given frame object.
             explicit StoredFrame(AnalysisDataStorageFrame *frame)
                 : frame(frame), status(eMissing)
             {
             }
-
+            //! Whether the frame has been started with startFrame().
             bool isStarted() const { return status >= eStarted; }
+            //! Whether the frame has been finished with finishFrame().
             bool isFinished() const { return status >= eFinished; }
+            //! Whether all notifications have been sent.
             bool isNotified() const { return status >= eNotified; }
+            //! Whether the frame is ready to be available outside the storage.
             bool isAvailable() const { return status >= eFinished; }
 
             /*! \brief
              * Actual frame data.
              *
-             * Always allocated.  Memory is managed by the parent Impl object
-             * to make the StoredFrame type STL-container-friendly.
+             * Never NULL.
              */
-            AnalysisDataStorageFrame *frame;
+            FramePointer              frame;
             //! In what state the frame currently is.
             Status                    status;
         };
index f3ae9163a7a8f03df2595870bfa797cf451427c4..f5f37b26fb0fd57c4366e871cd04328291ea9f69 100644 (file)
@@ -135,11 +135,6 @@ AnalysisDataStorage::Impl::Impl()
 
 AnalysisDataStorage::Impl::~Impl()
 {
-    FrameList::const_iterator i;
-    for (i = frames_.begin(); i != frames_.end(); ++i)
-    {
-        delete i->frame;
-    }
 }
 
 
@@ -331,7 +326,7 @@ AnalysisDataStorage::tryGetDataFrame(int index) const
     {
         return AnalysisDataFrameRef();
     }
-    const AnalysisDataStorageFrame *frame = storedFrame.frame;
+    const Impl::FramePointer &frame = storedFrame.frame;
     return AnalysisDataFrameRef(frame->header(), frame->values_);
 }
 
index d6e91e8cd8ebd73088dfbaea60b7da7ba76ed73c..48445f359fd09bfc64f4653f001dfafae966dfad 100644 (file)
@@ -72,6 +72,12 @@ class AnalysisDataStorage;
 class AnalysisDataStorageFrame
 {
     public:
+        /*! \brief Frees the frame object.
+         *
+         * Should not be called outside AnalysisDataStorage.
+         */
+        ~AnalysisDataStorageFrame();
+
         //! Returns header for the frame.
         const AnalysisDataFrameHeader &header() const { return header_; }
         //! Returns zero-based index of the frame.
@@ -184,7 +190,6 @@ class AnalysisDataStorageFrame
          */
         AnalysisDataStorageFrame(AnalysisDataStorage *storage, int columnCount,
                                  int index);
-        ~AnalysisDataStorageFrame();
 
         //! Clear all column values from the frame.
         void clearValues();
index c74d3a0465460f27954528dd8a1d779a4f569fc6..8211bacf58965afc303fe8d05c7b201d60dc32db 100644 (file)
@@ -40,8 +40,6 @@
  * \author Teemu Murtola <teemu.murtola@cbr.su.se>
  * \ingroup module_analysisdata
  */
-#include <memory>
-
 #include <gtest/gtest.h>
 
 #include "gromacs/analysisdata/arraydata.h"
index da473ac4913832ff6d6bd857603e0b43dee4be98..024042bcb7d70a89834f5ba61262b08e599df4db 100644 (file)
@@ -40,7 +40,7 @@
 
 #include "mock_module.h"
 
-#include <memory>
+#include <boost/scoped_ptr.hpp>
 
 namespace gmx
 {
@@ -55,16 +55,47 @@ namespace test
 class MockAnalysisModule::Impl
 {
     public:
+        //! Initializes a mock object with the given flags.
         explicit Impl(int flags);
 
+        /*! \brief
+         * Callback used to check frame start against reference data.
+         *
+         * Called to check parameters and order of calls to frameStarted().
+         * In addition to reference data checks, this method checks statically
+         * that the new frame matches \a frameIndex_.
+         */
         void startReferenceFrame(const AnalysisDataFrameHeader &header);
+        /*! \brief
+         * Callback used to check frame points against reference data.
+         *
+         * Called to check parameters and order of calls to pointsAdded().
+         */
         void checkReferencePoints(const AnalysisDataPointSetRef &points);
+        /*! \brief
+         * Callback used to check frame finish against reference data.
+         *
+         * Called to check parameters and order of calls to frameFinished().
+         * \a frameIndex_ is incremented here.
+         */
         void finishReferenceFrame(const AnalysisDataFrameHeader &header);
 
-        // Could be scoped_ptrs
-        std::auto_ptr<TestReferenceChecker>  rootChecker_;
-        std::auto_ptr<TestReferenceChecker>  frameChecker_;
+        /*! \brief
+         * Reference data checker to use for checking frames.
+         *
+         * Must be non-NULL if startReferenceFrame() is called.
+         */
+        boost::scoped_ptr<TestReferenceChecker>  rootChecker_;
+        /*! \brief
+         * Reference data checker to use to check the current frame.
+         *
+         * Non-NULL between startReferenceFrame() and finishReferenceFrame()
+         * calls.
+         */
+        boost::scoped_ptr<TestReferenceChecker>  frameChecker_;
+        //! Flags that will be returned by the mock module.
         int                     flags_;
+        //! Index of the current/next frame.
         int                     frameIndex_;
 };
 
index cf76d7bed178da6f4b241f4f7b7a1b9f95de90a2..aecca90b697e750086d3b8f105302a530b3de231 100644 (file)
@@ -80,7 +80,7 @@ void checkReferenceDataPoint(TestReferenceChecker *checker,
     }
 }
 
-}
+} // namespace
 
 MockAnalysisModule::Impl::Impl(int flags)
     : flags_(flags), frameIndex_(0)
@@ -91,6 +91,8 @@ MockAnalysisModule::Impl::Impl(int flags)
 void
 MockAnalysisModule::Impl::startReferenceFrame(const AnalysisDataFrameHeader &header)
 {
+    GMX_RELEASE_ASSERT(rootChecker_.get() != NULL,
+                       "Root checker not set, but reference data used");
     EXPECT_TRUE(frameChecker_.get() == NULL);
     EXPECT_EQ(frameIndex_, header.index());
     frameChecker_.reset(new TestReferenceChecker(
index 876a80deca1ae093b44357f61d810c09f40de9be..74ba36f4df43d0677141e6b17fab1f44c2c740fd 100644 (file)
 #ifndef GMX_OPTIONS_OPTIONS_IMPL_H
 #define GMX_OPTIONS_OPTIONS_IMPL_H
 
-#include <memory>
 #include <string>
 #include <vector>
 
+#include "gromacs/utility/uniqueptr.h"
+
 #include "options.h"
 
 namespace gmx
@@ -60,10 +61,12 @@ class AbstractOptionStorage;
 class Options::Impl
 {
     public:
+        //! Shorthand for a smart pointer to an option storage object.
+        typedef gmx_unique_ptr<AbstractOptionStorage>::type OptionPointer;
         //! Convenience type for list of sections.
         typedef std::vector<Options *> SubSectionList;
         //! Convenience type for list of options.
-        typedef std::vector<AbstractOptionStorage *> OptionList;
+        typedef std::vector<OptionPointer> OptionList;
 
         //! Sets the name and title.
         Impl(const char *name, const char *title);
index ba8b7ec5c4c9e657f304994dce7e3a19285edbcc..5ccd54f841b50ec5eca0de018aedc7c8d85f9fb0 100644 (file)
@@ -84,11 +84,6 @@ Options::Impl::Impl(const char *name, const char *title)
 
 Options::Impl::~Impl()
 {
-    OptionList::const_iterator i;
-    for (i = _options.begin(); i != _options.end(); ++i)
-    {
-        delete *i;
-    }
 }
 
 Options *Options::Impl::findSubSection(const char *name) const
@@ -111,7 +106,7 @@ AbstractOptionStorage *Options::Impl::findOption(const char *name) const
     {
         if ((*i)->name() == name)
         {
-            return *i;
+            return i->get();
         }
     }
     return NULL;
@@ -122,14 +117,14 @@ void Options::Impl::startSource()
     OptionList::const_iterator i;
     for (i = _options.begin(); i != _options.end(); ++i)
     {
-        AbstractOptionStorage *option = *i;
-        option->startSource();
+        AbstractOptionStorage &option = **i;
+        option.startSource();
     }
     SubSectionList::const_iterator j;
     for (j = _subSections.begin(); j != _subSections.end(); ++j)
     {
-        Options *section = *j;
-        section->_impl->startSource();
+        Options &section = **j;
+        section._impl->startSource();
     }
 }
 
@@ -180,13 +175,12 @@ void Options::addSubSection(Options *section)
 
 void Options::addOption(const AbstractOption &settings)
 {
-    std::auto_ptr<AbstractOptionStorage> option(settings.createDefaultStorage(this));
+    Impl::OptionPointer option(settings.createDefaultStorage(this));
     if (_impl->findOption(option->name().c_str()) != NULL)
     {
         GMX_THROW(APIError("Duplicate option: " + option->name()));
     }
-    _impl->_options.push_back(option.get());
-    option.release();
+    _impl->_options.push_back(move(option));
 }
 
 bool Options::isSet(const char *name) const
@@ -201,24 +195,24 @@ void Options::finish()
     Impl::OptionList::const_iterator i;
     for (i = _impl->_options.begin(); i != _impl->_options.end(); ++i)
     {
-        AbstractOptionStorage *option = *i;
+        AbstractOptionStorage &option = **i;
         try
         {
-            option->finish();
+            option.finish();
         }
         catch (const UserInputError &ex)
         {
-            MessageStringContext context(&errors, "In option " + option->name());
+            MessageStringContext context(&errors, "In option " + option.name());
             errors.append(ex.what());
         }
     }
     Impl::SubSectionList::const_iterator j;
     for (j = _impl->_subSections.begin(); j != _impl->_subSections.end(); ++j)
     {
-        Options *section = *j;
+        Options &section = **j;
         try
         {
-            section->finish();
+            section.finish();
         }
         catch (const UserInputError &ex)
         {
index c4815b5a5901cbf678cd69e364fc3cbe326cc9d7..5b0aa58d4600378a52fcfc21ebd390ceeead8741 100644 (file)
 #ifndef GMX_OPTIONS_OPTIONSTORAGETEMPLATE_H
 #define GMX_OPTIONS_OPTIONSTORAGETEMPLATE_H
 
-#include <memory>
 #include <string>
 #include <vector>
 
+#include <boost/scoped_ptr.hpp>
+
 #include "../fatalerror/exceptions.h"
 #include "../fatalerror/gmxassert.h"
 
@@ -238,8 +239,8 @@ class OptionStorageTemplate : public AbstractOptionStorage
         ValueList              *_values;
         T                      *_store;
         int                    *_countptr;
-        // Could be scoped_ptr
-        std::auto_ptr<T>        _defaultValueIfSet;
+        boost::scoped_ptr<ValueList> _ownedValues;
+        boost::scoped_ptr<T>    _defaultValueIfSet;
 
         // Copy and assign disallowed by base.
 };
@@ -255,14 +256,13 @@ OptionStorageTemplate<T>::OptionStorageTemplate(const OptionTemplate<T, U> &sett
       _store(settings._store),
       _countptr(settings._countptr)
 {
-    std::auto_ptr<std::vector<T> > valueGuard;
-    if (!_values)
+    if (_values == NULL)
     {
         // The flag should be set for proper error checking.
         GMX_RELEASE_ASSERT(!hasFlag(efExternalValueVector),
                            "Internal inconsistency");
-        valueGuard.reset(new std::vector<T>);
-        _values = valueGuard.get();
+        _ownedValues.reset(new std::vector<T>);
+        _values = _ownedValues.get();
     }
     if (hasFlag(efNoDefaultValue)
         && (settings._defaultValue != NULL
@@ -285,7 +285,7 @@ OptionStorageTemplate<T>::OptionStorageTemplate(const OptionTemplate<T, U> &sett
             // TODO: This is a bit hairy, as it indirectly calls a virtual function.
             commitValues();
         }
-        else if (!hasFlag(efExternalValueVector) && _store != NULL)
+        else if (_ownedValues.get() == NULL && _store != NULL)
         {
             _values->clear();
             int count = (settings.isVector() ?
@@ -305,17 +305,12 @@ OptionStorageTemplate<T>::OptionStorageTemplate(const OptionTemplate<T, U> &sett
         }
     }
     setFlag(efClearOnNextSet);
-    valueGuard.release();
 }
 
 
 template <typename T>
 OptionStorageTemplate<T>::~OptionStorageTemplate()
 {
-    if (!hasFlag(efExternalValueVector))
-    {
-        delete _values;
-    }
 }
 
 
index 6c5b30e75217bc08dfc8ea28a5e0f8a4e0a670b3..0867d277eb7e38603c4ccc96abb1c23e66fed585 100644 (file)
@@ -2571,7 +2571,7 @@ SelectionCompiler::compile(SelectionCollection *coll)
      */
     for (i = 0; i < sc->sel.size(); ++i)
     {
-        gmx::Selection *sel = sc->sel[i];
+        gmx::Selection *sel = sc->sel[i].get();
         init_pos_keyword_defaults(sel->rootElement_,
                                   coll->_impl->_spost.c_str(),
                                   coll->_impl->_rpost.c_str(),
index e6d1665cc7c97e465a5fbee4faa096b4838d52ef..a0210372df350d04cff50ccd406c63984ce4636d 100644 (file)
@@ -314,13 +314,12 @@ SelectionEvaluator::evaluate(SelectionCollection *coll,
         sel = sel->next;
     }
     /* Update selection information */
-    std::vector<Selection *>::iterator isel;
+    SelectionList::const_iterator isel;
     for (isel = sc->sel.begin(); isel != sc->sel.end(); ++isel)
     {
-        Selection *sel = *isel;
-
-        sel->refreshMassesAndCharges();
-        sel->updateCoveredFractionForFrame();
+        Selection &sel = **isel;
+        sel.refreshMassesAndCharges();
+        sel.updateCoveredFractionForFrame();
     }
 }
 
@@ -333,12 +332,12 @@ SelectionEvaluator::evaluateFinal(SelectionCollection *coll, int nframes)
 {
     gmx_ana_selcollection_t *sc = &coll->_impl->_sc;
 
-    std::vector<Selection *>::iterator isel;
+    SelectionList::const_iterator isel;
     for (isel = sc->sel.begin(); isel != sc->sel.end(); ++isel)
     {
-        Selection *sel = *isel;
-        sel->restoreOriginalPositions();
-        sel->computeAverageCoveredFraction(nframes);
+        Selection &sel = **isel;
+        sel.restoreOriginalPositions();
+        sel.computeAverageCoveredFraction(nframes);
     }
 }
 
index 29f5234de388a793fd97f8cb3a157ced1e6ac55f..44b156d1ba3f144f48997a19d2a7cf9871f5824f 100644 (file)
 #include <stdio.h>
 #include <stdarg.h>
 
-#include <memory>
-
 #include <futil.h>
 #include <smalloc.h>
 #include <string2.h>
@@ -1291,10 +1289,9 @@ _gmx_sel_append_selection(t_selelem *sel, t_selelem *last, yyscan_t scanner)
         /* Add the new selection to the collection if it is not a variable. */
         if (sel->child->type != SEL_SUBEXPR)
         {
-            std::auto_ptr<gmx::Selection> newsel(
-                new gmx::Selection(sel, _gmx_sel_lexer_pselstr(scanner)));
-            sc->sel.push_back(newsel.get());
-            newsel.release();
+            gmx::SelectionPointer selPtr(
+                    new gmx::Selection(sel, _gmx_sel_lexer_pselstr(scanner)));
+            sc->sel.push_back(move(selPtr));
         }
     }
     /* Clear the selection string now that we've saved it */
index 1eeeb3812c42e9b1125ed7487f99eaa0444bdcc8..4e1c522c70288d0fda6bb700169be1ce3643a301 100644 (file)
@@ -44,7 +44,9 @@
 #include <string>
 #include <vector>
 
-#include <typedefs.h>
+#include "../legacyheaders/typedefs.h"
+
+#include "gromacs/utility/uniqueptr.h"
 
 #include "../options/options.h"
 #include "../utility/flags.h"
 namespace gmx
 {
 class Selection;
+
+//! Smart pointer for managing a selection.
+typedef gmx_unique_ptr<Selection>::type SelectionPointer;
+//! Shorthand for storing a list of selections internally.
+typedef std::vector<SelectionPointer> SelectionList;
 }
 
 /*! \internal \brief
@@ -66,7 +73,7 @@ struct gmx_ana_selcollection_t
     /** Root of the selection element tree. */
     struct t_selelem           *root;
     /** Array of compiled selections. */
-    std::vector<gmx::Selection *>  sel;
+    gmx::SelectionList             sel;
     /** Number of variables defined. */
     int                            nvars;
     /** Selection strings for variables. */
@@ -113,8 +120,6 @@ class SelectionCollection::Impl
             SelectionOptionStorage     *storage;
         };
 
-        //! Shorthand for a list of selections stored internally.
-        typedef std::vector<Selection *> SelectionList;
         //! Shorthand for a list of selection requests.
         typedef std::vector<SelectionRequest> RequestList;
 
index e23b798d62f2738cd23508eac3d0afd7b7c9f35a..fe4268112a27af246f47cbb370ecc05abe632e89 100644 (file)
@@ -117,11 +117,7 @@ SelectionCollection::Impl::Impl(gmx_ana_poscalc_coll_t *pcc)
 SelectionCollection::Impl::~Impl()
 {
     _gmx_selelem_free_chain(_sc.root);
-    SelectionList::const_iterator isel;
-    for (isel = _sc.sel.begin(); isel != _sc.sel.end(); ++isel)
-    {
-        delete *isel;
-    }
+    _sc.sel.clear();
     for (int i = 0; i < _sc.nvars; ++i)
     {
         sfree(_sc.varstrs[i]);
@@ -177,7 +173,7 @@ SelectionCollection::Impl::runParser(yyscan_t scanner, int maxnr,
         SelectionList::const_iterator i;
         for (i = _sc.sel.begin() + oldCount; i != _sc.sel.end(); ++i)
         {
-            output->push_back(*i);
+            output->push_back(i->get());
         }
     }
 
index 92fe961d2e66e42214eaddffc6bdefb65d517d1b..f57408e2eab39b52820b06aae9726c3c8fadf6a6 100644 (file)
@@ -2,7 +2,7 @@ file(GLOB UTILITY_SOURCES *.cpp)
 set(LIBGROMACS_SOURCES ${LIBGROMACS_SOURCES} ${UTILITY_SOURCES} PARENT_SCOPE)
 
 set(UTILITY_PUBLIC_HEADERS
-    arrayref.h common.h flags.h format.h)
+    arrayref.h common.h flags.h format.h uniqueptr.h)
 install(FILES ${UTILITY_PUBLIC_HEADERS}
         DESTINATION ${INCL_INSTALL_DIR}/gromacs/utility
         COMPONENT development)
diff --git a/src/gromacs/utility/uniqueptr.h b/src/gromacs/utility/uniqueptr.h
new file mode 100644 (file)
index 0000000..b3a754f
--- /dev/null
@@ -0,0 +1,110 @@
+/*
+ *
+ *                This source code is part of
+ *
+ *                 G   R   O   M   A   C   S
+ *
+ *          GROningen MAchine for Chemical Simulations
+ *
+ * Written by David van der Spoel, Erik Lindahl, Berk Hess, and others.
+ * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
+ * Copyright (c) 2001-2009, The GROMACS development team,
+ * check out http://www.gromacs.org for more information.
+ *
+ * This program 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; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * If you want to redistribute modifications, please consider that
+ * scientific software is very special. Version control is crucial -
+ * bugs must be traceable. We will be happy to consider code for
+ * inclusion in the official distribution, but derived work must not
+ * be called official GROMACS. Details are found in the README & COPYING
+ * files - if they are missing, get the official version at www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the papers on the package - you can find them in the top README file.
+ *
+ * For more info, check our website at http://www.gromacs.org
+ */
+/*! \file
+ * \brief
+ * Declares gmx::gmx_unique_ptr and supporting functionality.
+ *
+ * \author Roland Schulz <roland@utk.edu>
+ * \author John Eblen <jeblen@acm.org>
+ * \inlibraryapi
+ * \ingroup module_utility
+ */
+#ifndef GMX_UTILITY_UNIQUEPTR_H
+#define GMX_UTILITY_UNIQUEPTR_H
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#ifdef HAVE_CXX11 // C++11 Compiler
+#include <memory>
+#include <utility>
+#else      // C++03 Compiler
+#include <boost/shared_ptr.hpp>
+#endif
+
+namespace gmx
+{
+
+/*! \class gmx_unique_ptr
+ * \brief
+ * Smart pointer for unique ownership.
+ *
+ * The \a type member typedef declares the actual smart pointer type.
+ * If std::unique_ptr from C++11 is available, it is used, otherwise maps to
+ * boost::shared_ptr. Because of this, there are some limitations to usage.
+ * gmx::move() should be used to move the pointer.
+ *
+ * Avoid using directly as a type, use a typedef instead. Typical usage:
+ * \code
+typedef gmx_unique_ptr<ExampleClass>::type ExampleClassPointer;
+ * \endcode
+ *
+ * \ingroup module_utility
+ * \inlibraryapi
+ */
+
+/*! \typedef gmx_unique_ptr::type
+ * \brief The smart pointer type.
+ * Work-around for the non-existence of template typedefs in C++03.
+ */
+
+/*! \fn boost::shared_ptr<T> &move(boost::shared_ptr<T> &ptr)
+ * \brief Moves gmx::gmx_unique_ptr type pointers
+ * For C++11 gmx::move is the std::move, for non-C++11 compilers, the
+ * move operation is a no-op.
+ *
+ * \ingroup module_utility
+ * \inlibraryapi
+ */
+#ifdef HAVE_CXX11 // C++11 Compiler
+using std::move;
+template<typename T>
+struct gmx_unique_ptr
+{
+    typedef std::unique_ptr<T> type;
+};
+#else // C++03 Compiler
+template<typename T>
+boost::shared_ptr<T> &move(boost::shared_ptr<T> &ptr)
+{
+    return ptr;
+}
+template<typename T>
+struct gmx_unique_ptr
+{
+    typedef boost::shared_ptr<T> type;
+};
+#endif
+
+} // namespace gmx
+
+#endif