Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement propagate_const in the framework #13093

Merged
merged 3 commits into from
Jan 29, 2016
Merged

Implement propagate_const in the framework #13093

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90459ea
Implement propagate_const
wmtan Jan 26, 2016
f6dbd80
Incorporate changes requested by Chris Jones
wmtan Jan 27, 2016
ff8dcc2
Declare timing report functions as const
wmtan Jan 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions DataFormats/Common/interface/CloningPtr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace edm {
template< class T, class P = ClonePolicy<T> >
class CloningPtr {
public:
CloningPtr(): ptr_(0) {}
CloningPtr(): ptr_(nullptr) {}
CloningPtr(const T& iPtr) : ptr_(P::clone(iPtr)) {}
CloningPtr(std::auto_ptr<T> iPtr) : ptr_(iPtr.release()) {}
CloningPtr(const CloningPtr<T,P>& iPtr) : ptr_(P::clone(*(iPtr.ptr_))) {}
Expand All @@ -48,11 +48,11 @@ namespace edm {
~CloningPtr() { delete ptr_;}

// ---------- const member functions ---------------------
T& operator*() const { return *ptr_; }
T& operator*() { return *ptr_; }

T* operator->() const { return ptr_; }
T* operator->() { return ptr_; }

T* get() const { return ptr_; }
T* get() { return ptr_; }

private:
T* ptr_;
Expand Down
3 changes: 1 addition & 2 deletions DataFormats/Common/interface/IndirectHolder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "DataFormats/Common/interface/BaseHolder.h"
#include "DataFormats/Common/interface/RefHolderBase.h"
#include "DataFormats/Provenance/interface/ProductID.h"
#include "FWCore/Utilities/interface/GCC11Compatibility.h"

#include <memory>

Expand All @@ -27,7 +26,7 @@ namespace edm {
// It may be better to use auto_ptr<RefHolderBase> in
// this constructor, so that the cloning can be avoided. I'm not
// sure if use of auto_ptr here causes any troubles elsewhere.
IndirectHolder() : BaseHolder<T>(), helper_( 0 ) { }
IndirectHolder() : BaseHolder<T>(), helper_( nullptr ) { }
IndirectHolder(std::shared_ptr<RefHolderBase> p);
template< typename U>
IndirectHolder(std::unique_ptr<U> p): helper_(p.release()) {}
Expand Down
5 changes: 2 additions & 3 deletions DataFormats/Common/interface/IndirectVectorHolder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "DataFormats/Common/interface/BaseVectorHolder.h"
#include "DataFormats/Common/interface/RefVectorHolderBase.h"
#include "DataFormats/Common/interface/IndirectHolder.h"
#include "FWCore/Utilities/interface/GCC11Compatibility.h"
#include <memory>

namespace edm {
Expand Down Expand Up @@ -55,7 +54,7 @@ namespace edm {

private:
typedef typename base_type::const_iterator_imp const_iterator_imp;
RefVectorHolderBase * helper_;
RefVectorHolderBase* helper_;

public:
struct const_iterator_imp_specific : public const_iterator_imp {
Expand Down Expand Up @@ -100,7 +99,7 @@ namespace edm {
};

template <typename T>
IndirectVectorHolder<T>::IndirectVectorHolder() : BaseVectorHolder<T>(), helper_( 0 ) { }
IndirectVectorHolder<T>::IndirectVectorHolder() : BaseVectorHolder<T>(), helper_( nullptr ) { }

template <typename T>
IndirectVectorHolder<T>::IndirectVectorHolder(std::shared_ptr<RefVectorHolderBase> p) :
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/Common/interface/ProductData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ is the storage unit of such information.
----------------------------------------------------------------------*/

#include "DataFormats/Provenance/interface/Provenance.h"
#include "FWCore/Utilities/interface/propagate_const.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include nor the one it replaced appears to be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dr15Jones Correct. It will be removed in my next commit to this PR.

#include <memory>

namespace edm {
Expand Down
8 changes: 4 additions & 4 deletions DataFormats/Common/interface/RefToBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ namespace edm {
template <class T>
inline
RefToBase<T>::RefToBase() :
holder_(0)
holder_(nullptr)
{ }

template <class T>
inline
RefToBase<T>::RefToBase(RefToBase const& other) :
holder_(other.holder_ ? other.holder_->clone() : 0)
holder_(other.holder_ ? other.holder_->clone() : nullptr)
{ }

template <class T>
Expand Down Expand Up @@ -234,7 +234,7 @@ namespace edm {
size_t
RefToBase<T>::key() const
{
if ( holder_ == 0 )
if ( holder_ == nullptr )
Exception::throwThis(errors::InvalidReference,
"attempting get key from null RefToBase;\n"
"You should check for nullity before calling key().");
Expand Down Expand Up @@ -370,7 +370,7 @@ namespace edm {
T const*
RefToBase<T>::getPtrImpl() const
{
return holder_ ? holder_->getPtr() : 0;
return holder_ ? holder_->getPtr() : nullptr;
}

template <class T>
Expand Down
18 changes: 9 additions & 9 deletions DataFormats/Common/interface/RefToBaseVector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ namespace edm {

value_type at(size_type idx) const;
value_type operator[](size_type idx) const;
bool isValid() const { return holder_ != 0; }
bool isInvalid() const { return holder_ == 0; }
bool isValid() const { return holder_ != nullptr; }
bool isInvalid() const { return holder_ == nullptr; }
bool empty() const;
size_type size() const;
//size_type capacity() const;
Expand All @@ -72,7 +72,7 @@ namespace edm {
CMS_CLASS_VERSION(10)

private:
holder_type * holder_;
holder_type* holder_;
};
}

Expand Down Expand Up @@ -109,7 +109,7 @@ namespace edm {
template <class T>
inline
RefToBaseVector<T>::RefToBaseVector() :
holder_(0)
holder_(nullptr)
{ }

template <class T>
Expand All @@ -122,7 +122,7 @@ namespace edm {
template <class T>
inline
RefToBaseVector<T>::RefToBaseVector(const RefToBaseVector<T>& iOther) :
holder_(iOther.holder_ ? iOther.holder_->clone() : 0)
holder_(iOther.holder_ ? iOther.holder_->clone() : nullptr)
{ }

template <class T>
Expand Down Expand Up @@ -159,7 +159,7 @@ namespace edm {
typename RefToBaseVector<T>::value_type
RefToBaseVector<T>::at(size_type idx) const
{
if ( holder_ == 0 )
if ( holder_ == nullptr )
Exception::throwThis( errors::InvalidReference,
"Trying to dereference null RefToBaseVector<T> in method: at(",
idx,
Expand Down Expand Up @@ -196,7 +196,7 @@ namespace edm {
void
RefToBaseVector<T>::clear()
{
if ( holder_ != 0 )
if ( holder_ != nullptr )
holder_->clear();
}

Expand All @@ -213,7 +213,7 @@ namespace edm {
EDProductGetter const *
RefToBaseVector<T>::productGetter() const
{
return holder_ ? holder_->productGetter() : 0;
return holder_ ? holder_->productGetter() : nullptr;
}

template <class T>
Expand Down Expand Up @@ -262,7 +262,7 @@ namespace edm {

template <typename T>
void RefToBaseVector<T>::push_back( const RefToBase<T> & r ) {
if ( holder_ == 0 ) {
if ( holder_ == nullptr ) {
std::auto_ptr<reftobase::BaseVectorHolder<T> > p = r.holder_->makeVectorHolder();
holder_ = p.release();
}
Expand Down
5 changes: 3 additions & 2 deletions DataFormats/Common/test/OwnArray_t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <memory>

#include "DataFormats/Common/interface/OwnArray.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"


struct Base
Expand All @@ -22,7 +23,7 @@ struct Derived : Base
void swap(Derived& other);
virtual Derived* clone() const;

int* pointer;
edm::propagate_const<int*> pointer;
};

Derived::Derived(int n) : pointer(new int(n)) { }
Expand All @@ -48,7 +49,7 @@ void swap(Derived& a, Derived& b)

Derived::~Derived()
{
delete pointer;
delete pointer.get();
}

Derived*
Expand Down
5 changes: 3 additions & 2 deletions DataFormats/Common/test/OwnVector_t.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <memory>

#include "DataFormats/Common/interface/OwnVector.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"


struct Base
Expand All @@ -22,7 +23,7 @@ struct Derived : Base
void swap(Derived& other);
virtual Derived* clone() const;

int* pointer;
edm::propagate_const<int*> pointer;
};

Derived::Derived(int n) : pointer(new int(n)) { }
Expand All @@ -48,7 +49,7 @@ void swap(Derived& a, Derived& b)

Derived::~Derived()
{
delete pointer;
delete pointer.get();
}

Derived*
Expand Down
3 changes: 2 additions & 1 deletion DataFormats/Common/test/SimpleEDProductGetter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

#include "DataFormats/Common/interface/EDProductGetter.h"
#include "DataFormats/Common/interface/Wrapper.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"

#include <map>
#include <memory>

class SimpleEDProductGetter : public edm::EDProductGetter {
public:

typedef std::map<edm::ProductID, std::shared_ptr<edm::WrapperBase> > map_t;
typedef std::map<edm::ProductID, edm::propagate_const<std::shared_ptr<edm::WrapperBase>>> map_t;

template<typename T>
void
Expand Down
3 changes: 2 additions & 1 deletion DataFormats/Common/test/testOwnArray.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <iterator>
#include <iostream>
#include "DataFormats/Common/interface/OwnArray.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"

class testOwnArray : public CppUnit::TestFixture {
CPPUNIT_TEST_SUITE(testOwnArray);
Expand All @@ -26,7 +27,7 @@ namespace testOA {
return value < o.value;
}
private:
bool * ref;
edm::propagate_const<bool*> ref;
};

struct DummyComp {
Expand Down
3 changes: 2 additions & 1 deletion DataFormats/Common/test/testOwnVector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <iostream>
#include "DataFormats/Common/interface/OwnVector.h"
#include "DataFormats/Common/interface/Ptr.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"

class testOwnVector : public CppUnit::TestFixture {
CPPUNIT_TEST_SUITE(testOwnVector);
Expand All @@ -27,7 +28,7 @@ namespace test {
return value < o.value;
}
private:
bool * ref;
edm::propagate_const<bool*> ref;
};

struct DummyComp {
Expand Down
5 changes: 3 additions & 2 deletions DataFormats/Provenance/interface/IndexIntoFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ The interface is too complex for general use.
#include "DataFormats/Provenance/interface/EventID.h"
#include "DataFormats/Provenance/interface/ProcessHistoryID.h"
#include "DataFormats/Provenance/interface/RunID.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"
#include "FWCore/Utilities/interface/value_ptr.h"

#include <memory>
Expand Down Expand Up @@ -1020,7 +1021,7 @@ namespace edm {
RunNumber_t currentRun_;
LuminosityBlockNumber_t currentLumi_;
EntryNumber_t numberOfEvents_;
std::shared_ptr<EventFinder> eventFinder_;
edm::propagate_const<std::shared_ptr<EventFinder>> eventFinder_;
std::vector<RunOrLumiIndexes> runOrLumiIndexes_;
std::vector<EventNumber_t> eventNumbers_;
std::vector<EventEntry> eventEntries_;
Expand All @@ -1042,7 +1043,7 @@ namespace edm {
void fillRunOrLumiIndexes() const;

void fillUnsortedEventNumbers() const;
void resetEventFinder() const {transient_.eventFinder_.reset();}
void resetEventFinder() const {transient_.eventFinder_ = nullptr;} // propagate_const<T> has no reset() function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we should investigate why transient_ is mutable.

std::vector<EventEntry>& eventEntries() const {return transient_.eventEntries_;}
std::vector<EventNumber_t>& eventNumbers() const {return transient_.eventNumbers_;}
void sortEvents() const;
Expand Down
5 changes: 3 additions & 2 deletions DataFormats/Provenance/interface/ProductHolderIndexHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ ProductRegistry is frozen.
#include "FWCore/Utilities/interface/ProductHolderIndex.h"
#include "FWCore/Utilities/interface/ProductKindOfType.h"
#include "FWCore/Utilities/interface/TypeID.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have prefered to just depend on propagate_const.h and only include propagate_const_safe in the cases where the stand alone function is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dr15Jones This will be done in my next commit


#include <iosfwd>
#include <memory>
Expand Down Expand Up @@ -334,9 +335,9 @@ namespace edm {
ProductHolderIndex index_;
};

std::unique_ptr<std::set<Item> > items_;
edm::propagate_const<std::unique_ptr<std::set<Item>>> items_;

std::unique_ptr<std::set<std::string> > processItems_;
edm::propagate_const<std::unique_ptr<std::set<std::string>>> processItems_;
};
}
#endif
2 changes: 1 addition & 1 deletion DataFormats/Provenance/interface/ProductProvenanceRetriever.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ProductProvenanceRetriever: Manages the per event/lumi/run per product provenanc
#include "DataFormats/Provenance/interface/BranchID.h"
#include "DataFormats/Provenance/interface/ProductProvenance.h"
#include "DataFormats/Provenance/interface/ProcessHistoryID.h"
#include "FWCore/Utilities/interface/propagate_const.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"

#include "tbb/concurrent_unordered_set.h"
#include <memory>
Expand Down
18 changes: 14 additions & 4 deletions DataFormats/Provenance/interface/ProductRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "DataFormats/Provenance/interface/BranchType.h"
#include "FWCore/Utilities/interface/ProductHolderIndex.h"
#include "FWCore/Utilities/interface/TypeID.h"
#include "FWCore/Utilities/interface/propagate_const_safe.h"

#include "boost/array.hpp"
#include <memory>
Expand Down Expand Up @@ -98,7 +99,8 @@ namespace edm {

bool anyProducts(BranchType const brType) const;

std::shared_ptr<ProductHolderIndexHelper> const& productLookup(BranchType branchType) const;
std::shared_ptr<ProductHolderIndexHelper const> productLookup(BranchType branchType) const;
std::shared_ptr<ProductHolderIndexHelper> productLookup(BranchType branchType);

// returns the appropriate ProductHolderIndex else ProductHolderIndexInvalid if no BranchID is available
ProductHolderIndex indexFrom(BranchID const& iID) const;
Expand Down Expand Up @@ -127,14 +129,22 @@ namespace edm {
struct Transients {
Transients();
void reset();

std::shared_ptr<ProductHolderIndexHelper const> eventProductLookup() const {return get_underlying_safe(eventProductLookup_);}
std::shared_ptr<ProductHolderIndexHelper>& eventProductLookup() {return get_underlying_safe(eventProductLookup_);}
std::shared_ptr<ProductHolderIndexHelper const> lumiProductLookup() const {return get_underlying_safe(lumiProductLookup_);}
std::shared_ptr<ProductHolderIndexHelper>& lumiProductLookup() {return get_underlying_safe(lumiProductLookup_);}
std::shared_ptr<ProductHolderIndexHelper const> runProductLookup() const {return get_underlying_safe(runProductLookup_);}
std::shared_ptr<ProductHolderIndexHelper>& runProductLookup() {return get_underlying_safe(runProductLookup_);}

bool frozen_;
// Is at least one (run), (lumi), (event) product produced this process?
boost::array<bool, NumBranchTypes> productProduced_;
bool anyProductProduced_;

std::shared_ptr<ProductHolderIndexHelper> eventProductLookup_;
std::shared_ptr<ProductHolderIndexHelper> lumiProductLookup_;
std::shared_ptr<ProductHolderIndexHelper> runProductLookup_;
edm::propagate_const<std::shared_ptr<ProductHolderIndexHelper>> eventProductLookup_;
edm::propagate_const<std::shared_ptr<ProductHolderIndexHelper>> lumiProductLookup_;
edm::propagate_const<std::shared_ptr<ProductHolderIndexHelper>> runProductLookup_;

ProductHolderIndex eventNextIndexValue_;
ProductHolderIndex lumiNextIndexValue_;
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/Provenance/src/IndexIntoFile.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace edm {
currentRun_ = invalidRun;
currentLumi_ = invalidLumi;
numberOfEvents_ = 0;
eventFinder_.reset();
eventFinder_ = nullptr; // propagate_const<T> has no reset() function
runOrLumiIndexes_.clear();
eventNumbers_.clear();
eventEntries_.clear();
Expand Down
Loading