Skip to content

Commit

Permalink
Merge pull request #79 from Dr15Jones/missingConsumesWarning
Browse files Browse the repository at this point in the history 
Print Error message when a module requests a data product without first registering the request via 'consumes'
  • Loading branch information
@ktf
ktf committed Jul 12, 2013
2 parents 575e6c3 + b263868 commit a76e0ff
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 31 deletions.
6 changes: 6 additions & 0 deletions FWCore/Framework/interface/EDConsumerBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ namespace edm {
void itemsToGet(BranchType, std::vector<ProductHolderIndex>&) const;
void itemsMayGet(BranchType, std::vector<ProductHolderIndex>&) const;


///\return true if the product corresponding to the index was registered via consumes or mayConsume call
bool registeredToConsume(ProductHolderIndex, BranchType) const;

///\return true of TypeID corresponds to a type specified in a consumesMany call
bool registeredToConsumeMany(TypeID const&, BranchType) const;
// ---------- static member functions --------------------

// ---------- member functions ---------------------------
Expand Down
16 changes: 11 additions & 5 deletions FWCore/Framework/interface/Principal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace edm {

class HistoryAppender;
class ProductHolderIndexHelper;
class EDConsumerBase;

struct FilledProductPtr {
bool operator()(boost::shared_ptr<ProductHolderBase> const& iObj) { return bool(iObj);}
Expand Down Expand Up @@ -105,13 +106,15 @@ namespace edm {

BasicHandle getByLabel(KindOfType kindOfType,
TypeID const& typeID,
InputTag const& inputTag) const;
InputTag const& inputTag,
EDConsumerBase const* consumes) const;

BasicHandle getByLabel(KindOfType kindOfType,
TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const;
std::string const& process,
EDConsumerBase const* consumes) const;

BasicHandle getByToken(KindOfType kindOfType,
TypeID const& typeID,
Expand All @@ -120,7 +123,8 @@ namespace edm {
bool& ambiguous) const;

void getManyByType(TypeID const& typeID,
BasicHandleVec& results) const;
BasicHandleVec& results,
EDConsumerBase const* consumes) const;

ProcessHistory const& processHistory() const {
return *processHistoryPtr_;
Expand Down Expand Up @@ -202,13 +206,15 @@ namespace edm {

ProductData const* findProductByLabel(KindOfType kindOfType,
TypeID const& typeID,
InputTag const& inputTag) const;
InputTag const& inputTag,
EDConsumerBase const* consumer) const;

ProductData const* findProductByLabel(KindOfType kindOfType,
TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const;
std::string const& process,
EDConsumerBase const* consumer) const;

// defaults to no-op unless overridden in derived class.
virtual void resolveProduct_(ProductHolderBase const&, bool /*fillOnDemand*/) const {}
Expand Down
46 changes: 46 additions & 0 deletions FWCore/Framework/src/EDConsumerBase.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,52 @@ EDConsumerBase::labelsForToken(EDGetToken iToken, Labels& oLabels) const
oLabels.process = oLabels.module+labels.m_deltaToProcessName;
}

bool
EDConsumerBase::registeredToConsume(ProductHolderIndex iIndex, BranchType iBranch) const
{
for(auto it = m_tokenInfo.begin<kLookupInfo>(),
itEnd = m_tokenInfo.end<kLookupInfo>();
it != itEnd; ++it) {
if(it->m_index == iIndex and
it->m_branchType == iBranch) {
return true;
}
}
//TEMPORARY: Remember so we do not have to do this again
//non thread-safe
EDConsumerBase* nonConstThis = const_cast<EDConsumerBase*>(this);
nonConstThis->m_tokenInfo.emplace_back(TokenLookupInfo{TypeID{},iIndex,iBranch},
true,
LabelPlacement{0,0,0},
PRODUCT_TYPE);

return false;
}

bool
EDConsumerBase::registeredToConsumeMany(TypeID const& iType, BranchType iBranch) const
{
for(auto it = m_tokenInfo.begin<kLookupInfo>(),
itEnd = m_tokenInfo.end<kLookupInfo>();
it != itEnd; ++it) {
//consumesMany entries do not have their index resolved
if(it->m_index == ProductHolderIndexInvalid and
it->m_type == iType and
it->m_branchType == iBranch) {
return true;
}
}
//TEMPORARY: Remember so we do not have to do this again
//non thread-safe
EDConsumerBase* nonConstThis = const_cast<EDConsumerBase*>(this);
nonConstThis->m_tokenInfo.emplace_back(TokenLookupInfo{iType,ProductHolderIndexInvalid,iBranch},
true,
LabelPlacement{0,0,0},
PRODUCT_TYPE);
return false;

}


void
EDConsumerBase::throwTypeMismatch(edm::TypeID const& iType, EDGetToken iToken) const
Expand Down
59 changes: 50 additions & 9 deletions FWCore/Framework/src/Principal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@
#include "FWCore/Framework/interface/DelayedReader.h"
#include "FWCore/Framework/interface/HistoryAppender.h"
#include "FWCore/Framework/interface/ProductDeletedException.h"
#include "FWCore/Framework/interface/EDConsumerBase.h"
#include "FWCore/Utilities/interface/Algorithms.h"
#include "FWCore/Utilities/interface/EDMException.h"
#include "FWCore/Utilities/interface/DictionaryTools.h"
#include "FWCore/Utilities/interface/ProductHolderIndex.h"
#include "FWCore/Utilities/interface/TypeID.h"
#include "FWCore/Utilities/interface/WrappedClassName.h"

#include "FWCore/MessageLogger/interface/MessageLogger.h"


#include <algorithm>
#include <cstring>
#include <limits>
Expand Down Expand Up @@ -98,7 +102,25 @@ namespace edm {
boost::shared_ptr<cms::Exception> exception = makeNotFoundException(where, PRODUCT_TYPE, productType, tag.label(), tag.instance(), tag.process());
throw *exception;
}


namespace {
void failedToRegisterConsumesMany(edm::TypeID const& iType) {
LogError("GetManyWithoutRegistration")<<"::getManyByType called for "<<iType<<" without a corresponding consumesMany being called for this module. \n";
}

void failedToRegisterConsumes(KindOfType kindOfType,
TypeID const& productType,
std::string const& moduleLabel,
std::string const& productInstanceName,
std::string const& processName) {
LogError("GetByLabelWithoutRegistration")<<"::getByLabel without corresponding call to consumes or mayConsumes for this module.\n"
<< (kindOfType == PRODUCT_TYPE ? " type: " : " type: edm::Veiw<")<<productType
<< (kindOfType == PRODUCT_TYPE ? "\n module label: " : ">\n module label: ")<<moduleLabel
<<"\n product instance name: '"<<productInstanceName
<<"'\n process name: '"<<processName<<"'\n";
}
}


Principal::Principal(boost::shared_ptr<ProductRegistry const> reg,
boost::shared_ptr<ProductHolderIndexHelper const> productLookup,
Expand Down Expand Up @@ -420,9 +442,10 @@ namespace edm {
BasicHandle
Principal::getByLabel(KindOfType kindOfType,
TypeID const& typeID,
InputTag const& inputTag) const {
InputTag const& inputTag,
EDConsumerBase const* consumer) const {

ProductData const* result = findProductByLabel(kindOfType, typeID, inputTag);
ProductData const* result = findProductByLabel(kindOfType, typeID, inputTag, consumer);
if(result == 0) {
boost::shared_ptr<cms::Exception> whyFailed =
makeNotFoundException("getByLabel", kindOfType, typeID, inputTag.label(), inputTag.instance(), inputTag.process());
Expand All @@ -436,9 +459,10 @@ namespace edm {
TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const {
std::string const& process,
EDConsumerBase const* consumer) const {

ProductData const* result = findProductByLabel(kindOfType, typeID, label, instance, process);
ProductData const* result = findProductByLabel(kindOfType, typeID, label, instance, process,consumer);
if(result == 0) {
boost::shared_ptr<cms::Exception> whyFailed =
makeNotFoundException("getByLabel", kindOfType, typeID, label, instance, process);
Expand Down Expand Up @@ -470,10 +494,15 @@ namespace edm {

void
Principal::getManyByType(TypeID const& typeID,
BasicHandleVec& results) const {
BasicHandleVec& results,
EDConsumerBase const* consumer) const {

assert(results.empty());

if(unlikely(consumer and (not consumer->registeredToConsumeMany(typeID,branchType())))) {
failedToRegisterConsumesMany(typeID);
}

// This finds the indexes to all the ProductHolder's matching the type
ProductHolderIndexHelper::Matches matches =
productLookup().relatedIndexes(PRODUCT_TYPE, typeID);
Expand Down Expand Up @@ -580,7 +609,8 @@ namespace edm {
ProductData const*
Principal::findProductByLabel(KindOfType kindOfType,
TypeID const& typeID,
InputTag const& inputTag) const {
InputTag const& inputTag,
EDConsumerBase const* consumer) const {

bool skipCurrentProcess = inputTag.willSkipCurrentProcess();

Expand Down Expand Up @@ -612,7 +642,11 @@ namespace edm {
}
inputTag.tryToCacheIndex(index, typeID, branchType(), &productRegistry());
}
if(unlikely( consumer and (not consumer->registeredToConsume(index,branchType())))) {
failedToRegisterConsumes(kindOfType,typeID,inputTag.label(),inputTag.instance(),inputTag.process());
}


boost::shared_ptr<ProductHolderBase> const& productHolder = productHolders_[index];

ProductHolderBase::ResolveStatus resolveStatus;
Expand All @@ -628,7 +662,8 @@ namespace edm {
TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const {
std::string const& process,
EDConsumerBase const* consumer) const {

ProductHolderIndex index = productLookup().index(kindOfType,
typeID,
Expand All @@ -647,6 +682,11 @@ namespace edm {
}
return 0;
}

if(unlikely( consumer and (not consumer->registeredToConsume(index,branchType())))) {
failedToRegisterConsumes(kindOfType,typeID,label,instance,process);
}

boost::shared_ptr<ProductHolderBase> const& productHolder = productHolders_[index];

ProductHolderBase::ResolveStatus resolveStatus;
Expand All @@ -662,7 +702,8 @@ namespace edm {
ProductData const* productData =
findProductByLabel(PRODUCT_TYPE,
typeID,
tag);
tag,
nullptr);
if(productData == nullptr) {
throwNotFoundException("findProductByTag", typeID, tag);
}
Expand Down
20 changes: 12 additions & 8 deletions FWCore/Framework/src/PrincipalGetAdapter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ namespace edm {
ModuleDescription const& md) :
//putProducts_(),
principal_(pcpl),
md_(md) {
md_(md),
consumer_(nullptr)
{
}

PrincipalGetAdapter::~PrincipalGetAdapter() {
Expand Down Expand Up @@ -99,7 +101,7 @@ namespace edm {
<< "() was called.\n"
<< "The index of the token was "<<token.index()<<".\n";
}

BasicHandle
PrincipalGetAdapter::makeFailToGetException(KindOfType kindOfType,
TypeID const& productType,
Expand Down Expand Up @@ -141,15 +143,15 @@ namespace edm {
BasicHandle
PrincipalGetAdapter::getByLabel_(TypeID const& typeID,
InputTag const& tag) const {
return principal_.getByLabel(PRODUCT_TYPE, typeID, tag);
return principal_.getByLabel(PRODUCT_TYPE, typeID, tag, consumer_);
}

BasicHandle
PrincipalGetAdapter::getByLabel_(TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const {
return principal_.getByLabel(PRODUCT_TYPE, typeID, label, instance, process);
return principal_.getByLabel(PRODUCT_TYPE, typeID, label, instance, process, consumer_);
}

BasicHandle
Expand All @@ -175,25 +177,27 @@ namespace edm {
BasicHandle
PrincipalGetAdapter::getMatchingSequenceByLabel_(TypeID const& typeID,
InputTag const& tag) const {
return principal_.getByLabel(ELEMENT_TYPE, typeID, tag);
return principal_.getByLabel(ELEMENT_TYPE, typeID, tag, consumer_);
}

BasicHandle
PrincipalGetAdapter::getMatchingSequenceByLabel_(TypeID const& typeID,
std::string const& label,
std::string const& instance,
std::string const& process) const {
return principal_.getByLabel(ELEMENT_TYPE,
auto h= principal_.getByLabel(ELEMENT_TYPE,
typeID,
label,
instance,
process);
process,
consumer_);
return h;
}

void
PrincipalGetAdapter::getManyByType_(TypeID const& tid,
BasicHandleVec& results) const {
principal_.getManyByType(tid, results);
principal_.getManyByType(tid, results, consumer_);
}

ProcessHistory const&
Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/test/Event_t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,10 +574,10 @@ void testEvent::getByLabel() {

}

BasicHandle bh = principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "LATE");
BasicHandle bh = principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "LATE",nullptr);
convert_handle(bh, h);
CPPUNIT_ASSERT(h->value == 100);
BasicHandle bh2(principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "nomatch"));
BasicHandle bh2(principal_->getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)), "modMulti", "int1", "nomatch",nullptr));
CPPUNIT_ASSERT(!bh2.isValid());

boost::shared_ptr<Wrapper<edmtest::IntProduct> const> ptr = getProductByTag<edmtest::IntProduct>(*principal_, inputTag);
Expand Down
4 changes: 2 additions & 2 deletions FWCore/Framework/test/eventprincipal_t.cppunit.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ void test_ep::failgetbyLabelTest() {

std::string label("this does not exist");

edm::BasicHandle h(pEvent_->getByLabel(edm::PRODUCT_TYPE, tid, label, std::string(), std::string()));
edm::BasicHandle h(pEvent_->getByLabel(edm::PRODUCT_TYPE, tid, label, std::string(), std::string(),nullptr));
CPPUNIT_ASSERT(h.failedToGet());
}

Expand All @@ -244,7 +244,7 @@ void test_ep::failgetManybyTypeTest() {
edm::TypeID tid(dummy);
std::vector<edm::BasicHandle > handles;

pEvent_->getManyByType(tid, handles);
pEvent_->getManyByType(tid, handles,nullptr);
CPPUNIT_ASSERT(handles.empty());
}

Expand Down
5 changes: 3 additions & 2 deletions FWCore/Modules/src/GetProductCheckerOutputModule.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,13 @@ namespace edm {
if(0 != oh.desc() && oh.desc()->branchID() != branchID) {
throw cms::Exception("BranchIDMissMatch") << "While processing " << id << " request for BranchID " << branchID << " returned BranchID " << oh.desc()->branchID() << "\n";
}

TypeID const& tid((*it)->branchDescription().unwrappedTypeID());
BasicHandle bh = p.getByLabel(PRODUCT_TYPE, tid,
(*it)->branchDescription().moduleLabel(),
(*it)->branchDescription().productInstanceName(),
(*it)->branchDescription().processName());
(*it)->branchDescription().processName(),
nullptr);

/*This doesn't appear to be an error, it just means the Product isn't available, which can be legitimate
if(!bh.product()) {
Expand Down
6 changes: 4 additions & 2 deletions IOPool/SecondaryInput/test/SecondaryProducer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ namespace edm {
BasicHandle bhandle = eventPrincipal.getByLabel(PRODUCT_TYPE, TypeID(typeid(edmtest::IntProduct)),
"EventNumber",
"",
"");
"",
nullptr);
assert(bhandle.isValid());
Handle<edmtest::IntProduct> handle;
convert_handle<edmtest::IntProduct>(bhandle, handle);
Expand All @@ -117,7 +118,8 @@ namespace edm {
BasicHandle bh = eventPrincipal.getByLabel(PRODUCT_TYPE, TypeID(typeid(TC)),
"Thing",
"",
"");
"",
nullptr);
assert(bh.isValid());
if(!(bh.interface()->dynamicTypeInfo() == typeid(TC))) {
handleimpl::throwConvertTypeError(typeid(TC), bh.interface()->dynamicTypeInfo());
Expand Down
Loading

0 comments on commit a76e0ff

Please sign in to comment.