From 6082e4a1a09bbb19a8eb3c06e359394ed3baf1a7 Mon Sep 17 00:00:00 2001 From: Roland Schulz Date: Fri, 13 Oct 2017 11:49:46 -0700 Subject: [PATCH] ArrayRef: Replace fromVector with subArray Creating ArrayRef from iterators is potentially dangerous, because it is incorrect for non-contiguous containers. arrayRefFromVector(v.begin()+start, v.begin()+start+length) is replaced with ArrayRef(v).subArray(start, length) Also: - Combine all conversion constructors Removes code duplication and makes conversion more powerful (e.g. base pointer or containers with allocators). - remove fromPointers and arrayRefFromPointers Wasn't used by any code - remove fromArray and replace wih arrayRefFromArray Change-Id: I05ad6b285ece58739d9f5bce48f9ecf4ade3454e --- src/gromacs/analysisdata/arraydata.cpp | 14 +- src/gromacs/analysisdata/dataframe.cpp | 8 +- src/gromacs/analysisdata/datastorage.cpp | 33 ++--- src/gromacs/analysisdata/framelocaldata.h | 7 +- src/gromacs/ewald/tests/pmegathertest.cpp | 18 +-- src/gromacs/ewald/tests/pmesplinespreadtest.cpp | 6 +- src/gromacs/ewald/tests/pmetestcommon.cpp | 6 +- src/gromacs/mdlib/simulationsignal.cpp | 4 +- src/gromacs/options/valuestore.h | 8 +- src/gromacs/selection/selection.h | 6 +- src/gromacs/selection/tests/nbsearch.cpp | 6 +- src/gromacs/utility/arrayref.h | 186 ++++++------------------ src/gromacs/utility/tests/arrayref.cpp | 47 ++++-- 13 files changed, 127 insertions(+), 222 deletions(-) diff --git a/src/gromacs/analysisdata/arraydata.cpp b/src/gromacs/analysisdata/arraydata.cpp index 60a5b172d5..bea6e7ad33 100644 --- a/src/gromacs/analysisdata/arraydata.cpp +++ b/src/gromacs/analysisdata/arraydata.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2010,2011,2012,2013,2014,2015, by the GROMACS development team, led by + * Copyright (c) 2010,2011,2012,2013,2014,2015,2017, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -72,11 +72,10 @@ AbstractAnalysisArrayData::tryGetDataFrameInternal(int index) const { return AnalysisDataFrameRef(); } - std::vector::const_iterator begin - = value_.begin() + index * columnCount(); return AnalysisDataFrameRef( AnalysisDataFrameHeader(index, xvalue(index), 0.0), - constArrayRefFromVector(begin, begin + columnCount()), + makeConstArrayRef(value_). + subArray(index * columnCount(), columnCount()), constArrayRefFromArray(&pointSetInfo_, 1)); } @@ -176,18 +175,17 @@ AbstractAnalysisArrayData::valuesReady() } bReady_ = true; - std::vector::const_iterator valueIter = value_.begin(); AnalysisDataModuleManager &modules = moduleManager(); modules.notifyDataStart(this); - for (int i = 0; i < rowCount(); ++i, valueIter += columnCount()) + for (int i = 0; i < rowCount(); ++i) { AnalysisDataFrameHeader header(i, xvalue(i), 0); modules.notifyFrameStart(header); modules.notifyPointsAdd( AnalysisDataPointSetRef( header, pointSetInfo_, - constArrayRefFromVector(valueIter, - valueIter + columnCount()))); + makeConstArrayRef(value_). + subArray(i*columnCount(), columnCount()))); modules.notifyFrameFinish(header); } modules.notifyDataFinish(); diff --git a/src/gromacs/analysisdata/dataframe.cpp b/src/gromacs/analysisdata/dataframe.cpp index e080f5f1ab..fcc104e467 100644 --- a/src/gromacs/analysisdata/dataframe.cpp +++ b/src/gromacs/analysisdata/dataframe.cpp @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2012,2013,2014, by the GROMACS development team, led by + * Copyright (c) 2012,2013,2014,2017, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -88,7 +88,7 @@ AnalysisDataPointSetRef::AnalysisDataPointSetRef( const AnalysisDataFrameHeader &header, const std::vector &values) : header_(header), dataSetIndex_(0), firstColumn_(0), - values_(constArrayRefFromVector(values.begin(), values.end())) + values_(values) { GMX_ASSERT(header_.isValid(), "Invalid point set reference should not be constructed"); @@ -168,8 +168,8 @@ AnalysisDataFrameRef::AnalysisDataFrameRef( const AnalysisDataFrameHeader &header, const std::vector &values, const std::vector &pointSets) - : header_(header), values_(constArrayRefFromVector(values.begin(), values.end())), - pointSets_(constArrayRefFromVector(pointSets.begin(), pointSets.end())) + : header_(header), values_(values), + pointSets_(pointSets) { GMX_ASSERT(!pointSets_.empty(), "There must always be a point set"); } diff --git a/src/gromacs/analysisdata/datastorage.cpp b/src/gromacs/analysisdata/datastorage.cpp index dc3db91b02..3c17937d9b 100644 --- a/src/gromacs/analysisdata/datastorage.cpp +++ b/src/gromacs/analysisdata/datastorage.cpp @@ -290,9 +290,6 @@ class AnalysisDataStorageImpl class AnalysisDataStorageFrameData { public: - //! Shorthand for a iterator into storage value containers. - typedef std::vector::const_iterator ValueIterator; - //! Indicates what operations have been performed on a frame. enum Status { @@ -355,7 +352,7 @@ class AnalysisDataStorageFrameData * Adds a new point set to this frame. */ void addPointSet(int dataSetIndex, int firstColumn, - ValueIterator begin, ValueIterator end); + ArrayRef v); /*! \brief * Finalizes the frame during AnalysisDataStorage::finishFrame(). * @@ -610,13 +607,12 @@ AnalysisDataStorageFrameData::startFrame( void AnalysisDataStorageFrameData::addPointSet(int dataSetIndex, int firstColumn, - ValueIterator begin, ValueIterator end) + ArrayRef v) { - const int valueCount = end - begin; + const int valueCount = v.size(); AnalysisDataPointSetInfo pointSetInfo(0, valueCount, dataSetIndex, firstColumn); - AnalysisDataPointSetRef pointSet(header(), pointSetInfo, - constArrayRefFromVector(begin, end)); + AnalysisDataPointSetRef pointSet(header(), pointSetInfo, v); storageImpl().modules_->notifyParallelPointsAdd(pointSet); if (storageImpl().shouldNotifyImmediately()) { @@ -626,7 +622,7 @@ AnalysisDataStorageFrameData::addPointSet(int dataSetIndex, int firstColumn, { pointSets_.emplace_back(values_.size(), valueCount, dataSetIndex, firstColumn); - std::copy(begin, end, std::back_inserter(values_)); + std::copy(v.begin(), v.end(), std::back_inserter(values_)); } } @@ -663,8 +659,7 @@ AnalysisDataStorageFrameData::pointSet(int index) const GMX_ASSERT(index >= 0 && index < pointSetCount(), "Invalid point set index"); return AnalysisDataPointSetRef( - header_, pointSets_[index], - constArrayRefFromVector(values_.begin(), values_.end())); + header_, pointSets_[index], values_); } } // namespace internal @@ -735,17 +730,15 @@ AnalysisDataStorageFrame::finishPointSet() "Should not be called for non-multipoint data"); if (bPointSetInProgress_) { - std::vector::const_iterator begin - = values_.begin() + currentOffset_; - std::vector::const_iterator end - = begin + columnCount_; - int firstColumn = 0; - while (begin != end && !begin->isSet()) + size_t begin = currentOffset_; + size_t end = begin + columnCount_; + int firstColumn = 0; + while (begin != end && !values_[begin].isSet()) { ++begin; ++firstColumn; } - while (end != begin && !(end-1)->isSet()) + while (end != begin && !values_[end-1].isSet()) { --end; } @@ -753,7 +746,9 @@ AnalysisDataStorageFrame::finishPointSet() { firstColumn = 0; } - data_->addPointSet(currentDataSet_, firstColumn, begin, end); + data_->addPointSet(currentDataSet_, firstColumn, + makeConstArrayRef(values_). + subArray(begin, end-begin)); } clearValues(); } diff --git a/src/gromacs/analysisdata/framelocaldata.h b/src/gromacs/analysisdata/framelocaldata.h index 99fb08c6ac..41f0039031 100644 --- a/src/gromacs/analysisdata/framelocaldata.h +++ b/src/gromacs/analysisdata/framelocaldata.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2014,2015, by the GROMACS development team, led by + * Copyright (c) 2014,2015,2017, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -134,9 +134,8 @@ class AnalysisDataFrameLocalDataHandle "Invalid data set index"); const int firstIndex = (*dataSetIndices_)[dataSet]; const int lastIndex = (*dataSetIndices_)[dataSet + 1]; - typename ValueArray::iterator begin = values_->begin() + firstIndex; - typename ValueArray::iterator end = values_->begin() + lastIndex; - return DataSetHandle(arrayRefFromVector(begin, end)); + return DataSetHandle(makeArrayRef(*values_). + subArray(firstIndex, lastIndex-firstIndex)); } //! Accesses a single value in the frame. ValueType &value(int dataSet, int column) diff --git a/src/gromacs/ewald/tests/pmegathertest.cpp b/src/gromacs/ewald/tests/pmegathertest.cpp index ef29c95475..f32d01820d 100644 --- a/src/gromacs/ewald/tests/pmegathertest.cpp +++ b/src/gromacs/ewald/tests/pmegathertest.cpp @@ -340,15 +340,13 @@ class PmeGatherTest : public ::testing::TestWithParam //! Sets the input atom data references once static void SetUpTestCase() { - auto gridLineIndicesIt = c_sampleGridLineIndicesFull.begin(); - auto chargesIt = c_sampleChargesFull.begin(); + size_t start = 0; for (auto atomCount : atomCounts) { AtomSizedData atomData; - atomData.gridLineIndices = GridLineIndicesVector::fromVector(gridLineIndicesIt, gridLineIndicesIt + atomCount); - gridLineIndicesIt += atomCount; - atomData.charges = ChargesVector::fromVector(chargesIt, chargesIt + atomCount); - chargesIt += atomCount; + atomData.gridLineIndices = GridLineIndicesVector(c_sampleGridLineIndicesFull).subArray(start, atomCount); + atomData.charges = ChargesVector(c_sampleChargesFull).subArray(start, atomCount); + start += atomCount; atomData.coordinates.resize(atomCount, RVec {1e6, 1e7, -1e8}); /* The coordinates are intentionally bogus in this test - only the size matters; the gridline indices are fed directly as inputs */ for (auto pmeOrder : pmeOrders) @@ -357,10 +355,8 @@ class PmeGatherTest : public ::testing::TestWithParam const size_t dimSize = atomCount * pmeOrder; for (int dimIndex = 0; dimIndex < DIM; dimIndex++) { - splineData.splineValues[dimIndex] = SplineParamsDimVector::fromVector(c_sampleSplineValuesFull.begin() + dimIndex * dimSize, - c_sampleSplineValuesFull.begin() + (dimIndex + 1) * dimSize); - splineData.splineDerivatives[dimIndex] = SplineParamsDimVector::fromVector(c_sampleSplineDerivativesFull.begin() + dimIndex * dimSize, - c_sampleSplineDerivativesFull.begin() + (dimIndex + 1) * dimSize); + splineData.splineValues[dimIndex] = SplineParamsDimVector(c_sampleSplineValuesFull).subArray(dimIndex * dimSize, dimSize); + splineData.splineDerivatives[dimIndex] = SplineParamsDimVector(c_sampleSplineDerivativesFull).subArray(dimIndex * dimSize, dimSize); } atomData.splineDataByPmeOrder[pmeOrder] = splineData; } @@ -430,7 +426,7 @@ class PmeGatherTest : public ::testing::TestWithParam /* Explicitly copying the sample forces to be able to modify them */ auto inputForcesFull(c_sampleForcesFull); GMX_RELEASE_ASSERT(inputForcesFull.size() >= atomCount, "Bad input forces size"); - auto forces = ForcesVector::fromVector(inputForcesFull.begin(), inputForcesFull.begin() + atomCount); + auto forces = ForcesVector(inputForcesFull).subArray(0, atomCount); /* Running the force gathering itself */ pmePerformGather(pmeSafe.get(), mode.first, inputForceTreatment, forces); diff --git a/src/gromacs/ewald/tests/pmesplinespreadtest.cpp b/src/gromacs/ewald/tests/pmesplinespreadtest.cpp index 3bb5917f35..9be27c35c4 100644 --- a/src/gromacs/ewald/tests/pmesplinespreadtest.cpp +++ b/src/gromacs/ewald/tests/pmesplinespreadtest.cpp @@ -281,11 +281,11 @@ static std::vector const c_sampleChargesFull 4.95f, 3.11f, 3.97f, 1.08f, 2.09f, 1.1f, 4.13f, 3.31f, 2.8f, 5.83f, 5.09f, 6.1f, 2.86f, 0.24f, 5.76f, 5.19f, 0.72f }; //! 1 charge -static auto const c_sampleCharges1 = ChargesVector::fromVector(c_sampleChargesFull.begin(), c_sampleChargesFull.begin() + 1); +static auto const c_sampleCharges1 = ChargesVector(c_sampleChargesFull).subArray(0, 1); //! 2 charges -static auto const c_sampleCharges2 = ChargesVector::fromVector(c_sampleChargesFull.begin() + 1, c_sampleChargesFull.begin() + 3); +static auto const c_sampleCharges2 = ChargesVector(c_sampleChargesFull).subArray(1, 2); //! 13 charges -static auto const c_sampleCharges13 = ChargesVector::fromVector(c_sampleChargesFull.begin() + 3, c_sampleChargesFull.begin() + 16); +static auto const c_sampleCharges13 = ChargesVector(c_sampleChargesFull).subArray(3, 13); //! Random coordinate vectors static CoordinatesVector const c_sampleCoordinatesFull diff --git a/src/gromacs/ewald/tests/pmetestcommon.cpp b/src/gromacs/ewald/tests/pmetestcommon.cpp index 0cd03e2da2..b16dbf4b0f 100644 --- a/src/gromacs/ewald/tests/pmetestcommon.cpp +++ b/src/gromacs/ewald/tests/pmetestcommon.cpp @@ -561,7 +561,7 @@ SplineParamsDimVector pmeGetSplineData(const gmx_pme_t *pme, CodePath mode, // fallthrough case CodePath::CPU: - result = SplineParamsDimVector::fromArray(sourceBuffer, dimSize); + result = arrayRefFromArray(sourceBuffer, dimSize); break; default: @@ -581,11 +581,11 @@ GridLineIndicesVector pmeGetGridlineIndices(const gmx_pme_t *pme, CodePath mode) switch (mode) { case CodePath::CUDA: - gridLineIndices = GridLineIndicesVector::fromArray(reinterpret_cast(pme->gpu->staging.h_gridlineIndices), atomCount); + gridLineIndices = arrayRefFromArray(reinterpret_cast(pme->gpu->staging.h_gridlineIndices), atomCount); break; case CodePath::CPU: - gridLineIndices = GridLineIndicesVector::fromArray(reinterpret_cast(atc->idx), atomCount); + gridLineIndices = arrayRefFromArray(reinterpret_cast(atc->idx), atomCount); break; default: diff --git a/src/gromacs/mdlib/simulationsignal.cpp b/src/gromacs/mdlib/simulationsignal.cpp index 95afbb7b68..4def33c345 100644 --- a/src/gromacs/mdlib/simulationsignal.cpp +++ b/src/gromacs/mdlib/simulationsignal.cpp @@ -3,7 +3,7 @@ * * Copyright (c) 1991-2000, University of Groningen, The Netherlands. * Copyright (c) 2001-2004, The GROMACS development team. - * Copyright (c) 2013,2014,2015,2016, by the GROMACS development team, led by + * Copyright (c) 2013,2014,2015,2016,2017, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -83,7 +83,7 @@ SimulationSignaller::getCommunicationBuffer() std::transform(std::begin(*signals_), std::end(*signals_), std::begin(mpiBuffer_), [](const SimulationSignals::value_type &s) { return s.sig; }); - return gmx::ArrayRef::fromArray(mpiBuffer_.data(), mpiBuffer_.size()); + return mpiBuffer_; } else { diff --git a/src/gromacs/options/valuestore.h b/src/gromacs/options/valuestore.h index d19652adec..1b9573a099 100644 --- a/src/gromacs/options/valuestore.h +++ b/src/gromacs/options/valuestore.h @@ -1,7 +1,7 @@ /* * This file is part of the GROMACS molecular simulation package. * - * Copyright (c) 2016, by the GROMACS development team, led by + * Copyright (c) 2016,2017, by the GROMACS development team, led by * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, * and including many others, as listed in the AUTHORS file in the * top-level source directory and at http://www.gromacs.org. @@ -60,7 +60,7 @@ class OptionValueStorePlain : public IOptionValueStore } virtual int valueCount() { return count_; } - virtual ArrayRef values() { return ArrayRef::fromArray(store_, count_); } + virtual ArrayRef values() { return arrayRefFromArray(store_, count_); } virtual void clear() { count_ = 0; @@ -124,8 +124,8 @@ class OptionValueStoreVector : public IOptionValueStore virtual int valueCount() { return static_cast(store_->size()); } virtual ArrayRef values() { - return ArrayRef::fromArray(reinterpret_cast(boolStore_.data()), - boolStore_.size()); + return arrayRefFromArray(reinterpret_cast(boolStore_.data()), + boolStore_.size()); } virtual void clear() { diff --git a/src/gromacs/selection/selection.h b/src/gromacs/selection/selection.h index 51424d8811..80eb0900e0 100644 --- a/src/gromacs/selection/selection.h +++ b/src/gromacs/selection/selection.h @@ -383,8 +383,7 @@ class Selection // (and thus the masses and charges are fixed). GMX_ASSERT(data().posMass_.size() >= static_cast(posCount()), "Internal inconsistency"); - return constArrayRefFromVector(data().posMass_.begin(), - data().posMass_.begin() + posCount()); + return makeArrayRef(data().posMass_).subArray(0, posCount()); } //! Returns charges for this selection as a continuous array. ArrayRef charges() const @@ -394,8 +393,7 @@ class Selection // (and thus the masses and charges are fixed). GMX_ASSERT(data().posCharge_.size() >= static_cast(posCount()), "Internal inconsistency"); - return constArrayRefFromVector(data().posCharge_.begin(), - data().posCharge_.begin() + posCount()); + return makeArrayRef(data().posCharge_).subArray(0, posCount()); } /*! \brief * Returns reference IDs for this selection as a continuous array. diff --git a/src/gromacs/selection/tests/nbsearch.cpp b/src/gromacs/selection/tests/nbsearch.cpp index 78ee37a17a..105a3a5725 100644 --- a/src/gromacs/selection/tests/nbsearch.cpp +++ b/src/gromacs/selection/tests/nbsearch.cpp @@ -331,13 +331,11 @@ class ExclusionsHelper gmx::ArrayRef refPosIds() const { - return gmx::constArrayRefFromVector(exclusionIds_.begin(), - exclusionIds_.begin() + refPosCount_); + return gmx::makeArrayRef(exclusionIds_).subArray(0, refPosCount_); } gmx::ArrayRef testPosIds() const { - return gmx::constArrayRefFromVector(exclusionIds_.begin(), - exclusionIds_.begin() + testPosCount_); + return gmx::makeArrayRef(exclusionIds_).subArray(0, testPosCount_); } private: diff --git a/src/gromacs/utility/arrayref.h b/src/gromacs/utility/arrayref.h index a411e1903b..193134d8f0 100644 --- a/src/gromacs/utility/arrayref.h +++ b/src/gromacs/utility/arrayref.h @@ -111,8 +111,6 @@ struct EmptyArrayRef {}; template class ArrayRef { - private: - typedef typename std::remove_const::type non_const_value_type; public: //! Type of values stored in the container. typedef T value_type; @@ -138,59 +136,6 @@ class ArrayRef typedef std::reverse_iterator const_reverse_iterator; /*! \brief - * Constructs a reference to a particular range from two pointers. - * - * \param[in] begin Pointer to the beginning of a range. - * \param[in] end Pointer to the end of a range. - * - * Passed pointers must remain valid for the lifetime of this object. - */ - static ArrayRef - fromPointers(value_type *begin, value_type *end) - { - return ArrayRef(begin, end); - } - /*! \brief - * Constructs a reference to an array. - * - * \param[in] begin Pointer to the beginning of the array. - * May be NULL if \p size is zero. - * \param[in] size Number of elements in the array. - * - * Passed pointer must remain valid for the lifetime of this object. - */ - static ArrayRef - fromArray(value_type *begin, size_t size) - { - return ArrayRef(begin, begin+size); - } - /*! \brief - * Constructs a reference to a particular range in a std::vector. - * - * \param[in] begin Iterator to the beginning of a range. - * \param[in] end Iterator to the end of a range. - * - * The referenced vector must remain valid and not be reallocated for - * the lifetime of this object. - */ - static ArrayRef - fromVector(typename std::vector::iterator begin, - typename std::vector::iterator end) - { - value_type *p_begin = (begin != end) ? &*begin : nullptr; - value_type *p_end = p_begin + (end-begin); - return ArrayRef(p_begin, p_end); - } - //! \copydoc ArrayRef::fromVector(typename std::vector::iterator, typename std::vector::iterator) - static ArrayRef - fromVector(typename std::vector::const_iterator begin, - typename std::vector::const_iterator end) - { - value_type *p_begin = (begin != end) ? &*begin : nullptr; - value_type *p_end = p_begin + (end-begin); - return ArrayRef(p_begin, p_end); - } - /*! \brief * Constructs an empty reference. */ ArrayRef() : begin_(NULL), end_(NULL) {} @@ -203,14 +148,25 @@ class ArrayRef */ ArrayRef(const EmptyArrayRef &) : begin_(nullptr), end_(nullptr) {} /*! \brief - * Constructs a reference to const data from a reference to non-const data. + * Constructs a reference to a container or reference + * + * \param[in] o container to reference. * - * Constructs a ArrayRef from a ArrayRef. + * Can be used to create a reference to a whole vector, std::array or + * an ArrayRef. The destination has to have a convertible pointer type + * (identical besides const or base class). + * + * Passed container must remain valid and not be reallocated for the + * lifetime of this object. + * + * This constructor is not explicit to allow directly passing + * a container to a method that takes ArrayRef. */ - template //Otherwise useless template argument - //to avoid this being used as copy constructor - ArrayRef(const ArrayRef &o) : - begin_(o.begin()), end_(o.end()) {} + template::type::pointer, + pointer>::value>::type> + ArrayRef(U &&o) : begin_(o.data()), end_(o.data()+o.size()) {} /*! \brief * Constructs a reference to a particular range. * @@ -218,63 +174,12 @@ class ArrayRef * \param[in] end Pointer to the end of a range. * * Passed pointers must remain valid for the lifetime of this object. - * - * \note For clarity, use the non-member function arrayRefFromPointers - * instead. */ ArrayRef(pointer begin, pointer end) : begin_(begin), end_(end) { GMX_ASSERT(end >= begin, "Invalid range"); } - /*! \brief - * Constructs a reference to a whole std::vector. - * - * \param[in] v Vector to reference. - * - * Passed vector must remain valid and not be reallocated for the - * lifetime of this object. - * - * This constructor is not explicit to allow directly passing - * std::vector to a method that takes ArrayRef. - */ - template - ArrayRef(std::vector &v) - : begin_((!v.empty()) ? &v[0] : nullptr), - end_((!v.empty()) ? &v[0] + v.size() : nullptr) - { - } - //! \copydoc ArrayRef::ArrayRef(std::vector&) - template - ArrayRef(const std::vector &v) - : begin_((!v.empty()) ? &v[0] : nullptr), - end_((!v.empty()) ? &v[0] + v.size() : nullptr) - { - } - /*! \brief - * Constructs a reference to a whole std::array. - * - * \param[in] a Array to reference. - * - * Passed array must remain valid for the lifetime of this - * object. - * - * This constructor is not explicit to allow directly passing - * std::array to a method that takes ArrayRef. - */ - template - ArrayRef(std::array &a) - : begin_((!a.empty()) ? &a[0] : NULL), - end_((!a.empty()) ? &a[0] + a.size() : NULL) - { - } - //! \copydoc ArrayRef::ArrayRef(std::array &) - template - ArrayRef(const std::array &a) - : begin_((!a.empty()) ? &a[0] : NULL), - end_((!a.empty()) ? &a[0] + a.size() : NULL) - { - } //! \cond // Doxygen 1.8.5 doesn't parse the declaration correctly... /*! \brief @@ -300,6 +205,11 @@ class ArrayRef } //! \endcond + //! Returns a reference to part of the container. + ArrayRef subArray(size_type start, size_type count) const + { + return {begin_+start, begin_+start+count}; + } //! Returns an iterator to the beginning of the container. iterator begin() const { return begin_; } //! Returns an iterator to the end of the container. @@ -352,51 +262,45 @@ class ArrayRef pointer end_; }; -//! \copydoc ArrayRef::fromPointers() -//! \related ArrayRef -template -ArrayRef arrayRefFromPointers(T *begin, T *end) -{ - return ArrayRef::fromPointers(begin, end); -} //! \copydoc ArrayRef::fromArray() //! \related ArrayRef template ArrayRef arrayRefFromArray(T *begin, size_t size) { - return ArrayRef::fromArray(begin, size); + return ArrayRef(begin, begin+size); } -//! \copydoc ArrayRef::fromVector() -//! \related ArrayRef -template -ArrayRef arrayRefFromVector(typename std::vector::iterator begin, - typename std::vector::iterator end) -{ - return ArrayRef::fromVector(begin, end); -} - -//! \copydoc ArrayRef::fromPointers() +//! \copydoc ArrayRef::fromArray() //! \related ArrayRef template -ArrayRef constArrayRefFromPointers(const T *begin, const T *end) +ArrayRef constArrayRefFromArray(const T *begin, size_t size) { - return ArrayRef::fromPointers(begin, end); + return ArrayRef(begin, begin+size); } -//! \copydoc ArrayRef::fromArray() -//! \related ArrayRef + +/*! \brief + * Create ArrayRef from container with type deduction + * + * \see ArrayRef + */ template -ArrayRef constArrayRefFromArray(const T *begin, size_t size) +ArrayRef::value, + const typename T::value_type, + typename T::value_type>::type> +makeArrayRef(T &c) { - return ArrayRef::fromArray(begin, size); + return c; } -//! \copydoc ArrayRef::fromVector() -//! \related ArrayRef + +/*! \brief + * Create ArrayRef to const T from container with type deduction + * + * \see ArrayRef + */ template -ArrayRef constArrayRefFromVector(typename std::vector::const_iterator begin, - typename std::vector::const_iterator end) +ArrayRef makeConstArrayRef(const T &c) { - return ArrayRef::fromVector(begin, end); + return c; } /*! \brief diff --git a/src/gromacs/utility/tests/arrayref.cpp b/src/gromacs/utility/tests/arrayref.cpp index 7130248256..3b85161879 100644 --- a/src/gromacs/utility/tests/arrayref.cpp +++ b/src/gromacs/utility/tests/arrayref.cpp @@ -149,6 +149,15 @@ TYPED_TEST_CASE(ArrayRefTest, ArrayRefTypes); }; \ size_t (aSize) = sizeof((a)) / sizeof(typename TestFixture::ValueType); +#define DEFINE_NON_CONST_ARRAY(a, aSize) \ + typename TestFixture::NonConstValueType (a)[] = { \ + static_cast(1.2), \ + static_cast(2.4), \ + static_cast(3.1) \ + }; \ + size_t (aSize) = sizeof((a)) / sizeof(typename TestFixture::ValueType); + + TYPED_TEST(ArrayRefTest, MakeWithAssignmentWorks) { DEFINE_ARRAY(a, aSize); @@ -156,6 +165,13 @@ TYPED_TEST(ArrayRefTest, MakeWithAssignmentWorks) this->runTests(a, aSize, a, arrayRef); } +TYPED_TEST(ArrayRefTest, MakeWithNonConstAssignmentWorks) +{ + DEFINE_NON_CONST_ARRAY(a, aSize); + typename TestFixture::ArrayRefType arrayRef = a; + this->runTests(a, aSize, a, arrayRef); +} + TYPED_TEST(ArrayRefTest, ConstructWithTemplateConstructorWorks) { DEFINE_ARRAY(a, aSize); @@ -163,43 +179,44 @@ TYPED_TEST(ArrayRefTest, ConstructWithTemplateConstructorWorks) this->runTests(a, aSize, a, arrayRef); } -TYPED_TEST(ArrayRefTest, ConstructFromPointersWorks) +TYPED_TEST(ArrayRefTest, ConstructWithNonConstTemplateConstructorWorks) { - DEFINE_ARRAY(a, aSize); - typename TestFixture::ArrayRefType arrayRef(a, a + aSize); + DEFINE_NON_CONST_ARRAY(a, aSize); + typename TestFixture::ArrayRefType arrayRef(a); this->runTests(a, aSize, a, arrayRef); } -TYPED_TEST(ArrayRefTest, MakeFromPointersWorks) +TYPED_TEST(ArrayRefTest, ConstructFromPointersWorks) { DEFINE_ARRAY(a, aSize); - typename TestFixture::ArrayRefType arrayRef - = TestFixture::ArrayRefType::fromPointers(a, a + aSize); + typename TestFixture::ArrayRefType arrayRef(a, a + aSize); this->runTests(a, aSize, a, arrayRef); } -TYPED_TEST(ArrayRefTest, MakeFromArrayWorks) +TYPED_TEST(ArrayRefTest, ConstructFromNonConstPointersWorks) { - DEFINE_ARRAY(a, aSize); - typename TestFixture::ArrayRefType arrayRef - = TestFixture::ArrayRefType::fromArray(a, aSize); + DEFINE_NON_CONST_ARRAY(a, aSize); + typename TestFixture::ArrayRefType arrayRef(a, a + aSize); this->runTests(a, aSize, a, arrayRef); } +template +using makeConstIf_t = typename std::conditional::type; + TYPED_TEST(ArrayRefTest, ConstructFromVectorWorks) { DEFINE_ARRAY(a, aSize); - std::vector v(a, a + aSize); - typename TestFixture::ArrayRefType arrayRef(v); + makeConstIf_t::value, + std::vector > v(a, a + aSize); + typename TestFixture::ArrayRefType arrayRef(v); this->runTests(a, v.size(), v.data(), arrayRef); } -TYPED_TEST(ArrayRefTest, MakeFromVectorWorks) +TYPED_TEST(ArrayRefTest, ConstructFromNonConstVectorWorks) { DEFINE_ARRAY(a, aSize); std::vector v(a, a + aSize); - typename TestFixture::ArrayRefType arrayRef - = TestFixture::ArrayRefType::fromVector(v.begin(), v.end()); + typename TestFixture::ArrayRefType arrayRef(v); this->runTests(a, v.size(), v.data(), arrayRef); } -- 2.11.4.GIT