Skip to content

Commit 8e38467

Browse files
committed
Changes based on code review
- fix for clang compilation (class->struct) - make code more readable/idiomatic
1 parent 0ea1997 commit 8e38467

File tree

5 files changed

+26
-24
lines changed

5 files changed

+26
-24
lines changed

FWCore/Framework/interface/maker/ModuleHolder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace edm {
4242
class ProductRegistry;
4343
class ThinnedAssociationsHelper;
4444
class ModuleConsumesInfo;
45-
class ModuleConsumesMinimalESInfo;
45+
struct ModuleConsumesMinimalESInfo;
4646
namespace maker {
4747
class ModuleHolder {
4848
public:

FWCore/Framework/src/PathsAndConsumesOfModules.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ namespace edm {
143143
modulesWhoseProductsAreConsumedBy[iBranchType].clear();
144144
}
145145

146+
//The maxModuleID will be used as an index so we need the +1 to accomodate that
146147
allModuleDescriptions.reserve(moduleRegistry.maxModuleID() + 1);
147148
moduleIDToHolder.resize(moduleRegistry.maxModuleID() + 1);
148149
for (auto iBranchType = 0U; iBranchType < NumBranchTypes; ++iBranchType) {

FWCore/Framework/src/Schedule.cc

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,6 @@ namespace edm {
423423
*moduleRegistry_, proc_pset, *pathNames_, *endPathNames_, prealloc, preg, *areg, processConfiguration);
424424
resultsInserter_ = std::move(builder.resultsInserter_);
425425
assert(builder.pathStatusInserters_.size() == builder.pathNameAndModules_.size());
426-
assert(builder.endPathStatusInserters_.size() == builder.endpathNameAndModules_.size() or
427-
(builder.endpathNameAndModules_.size() == 1 and builder.endPathStatusInserters_.empty()));
428426
pathStatusInserters_ = std::move(builder.pathStatusInserters_);
429427
endPathStatusInserters_ = std::move(builder.endPathStatusInserters_);
430428

@@ -437,15 +435,17 @@ namespace edm {
437435
}
438436
std::vector<StreamSchedule::EndPathInfo> endpaths;
439437
endpaths.reserve(builder.endpathNameAndModules_.size());
440-
index = 0;
441-
for (auto& path : builder.endpathNameAndModules_) {
442-
if (endPathStatusInserters_.empty()) {
443-
endpaths.emplace_back(path.first, std::move(path.second), std::shared_ptr<EndPathStatusInserter>());
444-
445-
} else {
438+
if (builder.endpathNameAndModules_.size() == 1) {
439+
assert(endPathStatusInserters_.empty());
440+
auto& path = builder.endpathNameAndModules_.front();
441+
endpaths.emplace_back(path.first, std::move(path.second), std::shared_ptr<EndPathStatusInserter>());
442+
} else {
443+
assert(endPathStatusInserters_.size() == builder.endpathNameAndModules_.size());
444+
index = 0;
445+
for (auto& path : builder.endpathNameAndModules_) {
446446
endpaths.emplace_back(path.first, std::move(path.second), get_underlying_safe(endPathStatusInserters_[index]));
447+
++index;
447448
}
448-
++index;
449449
}
450450

451451
assert(0 < prealloc.numberOfStreams());

FWCore/Framework/src/ScheduleBuilder.cc

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ namespace edm {
462462
iProcessConfiguration);
463463
if (module == nullptr) {
464464
std::string pathType("endpath");
465-
if (std::count(iEndPathNames.begin(), iEndPathNames.end(), iPathName) == 0) {
465+
if (std::find(iEndPathNames.begin(), iEndPathNames.end(), iPathName) == iEndPathNames.end()) {
466466
pathType = std::string("path");
467467
}
468468
throw Exception(errors::Configuration)
@@ -476,8 +476,8 @@ namespace edm {
476476
// See if the filter is allowed.
477477
std::vector<std::string> allowed_filters =
478478
ioProcessPSet.getUntrackedParameter<std::vector<std::string>>("@filters_on_endpaths");
479-
if (std::count(allowed_filters.begin(), allowed_filters.end(), module->moduleDescription().moduleName()) ==
480-
0) {
479+
if (std::find(allowed_filters.begin(), allowed_filters.end(), module->moduleDescription().moduleName()) ==
480+
allowed_filters.end()) {
481481
// Filter is not allowed. Ignore the result, and issue a warning.
482482
filterAction = WorkerInPath::Ignore;
483483
LogWarning("FilterOnEndPath")
@@ -686,15 +686,17 @@ namespace edm {
686686
}
687687
}
688688
if (!shouldBeUsedLabels.empty()) {
689-
std::ostringstream unusedStream;
690-
unusedStream << "'" << shouldBeUsedLabels.front() << "'";
691-
for (std::vector<std::string>::iterator itLabel = shouldBeUsedLabels.begin() + 1,
692-
itLabelEnd = shouldBeUsedLabels.end();
693-
itLabel != itLabelEnd;
694-
++itLabel) {
695-
unusedStream << ",'" << *itLabel << "'";
696-
}
697-
LogInfo("path") << "The following module labels are not assigned to any path:\n" << unusedStream.str() << "\n";
689+
LogInfo("path").log([&shouldBeUsedLabels](auto& l) {
690+
l << "The following module labels are not assigned to any path:\n";
691+
l << "'" << shouldBeUsedLabels.front() << "'";
692+
for (std::vector<std::string>::iterator itLabel = shouldBeUsedLabels.begin() + 1,
693+
itLabelEnd = shouldBeUsedLabels.end();
694+
itLabel != itLabelEnd;
695+
++itLabel) {
696+
l << ",'" << *itLabel << "'";
697+
}
698+
l << "\n";
699+
});
698700
}
699701
}
700702

FWCore/Framework/src/StreamSchedule.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ namespace edm {
156156
: workerManagerRuns_(modReg, areg, actions),
157157
workerManagerLumisAndEvents_(modReg, areg, actions),
158158
actReg_(areg),
159-
results_(new HLTGlobalStatus(paths.size())),
159+
results_(std::make_shared<HLTGlobalStatus>(paths.size())),
160160
results_inserter_(),
161161
trig_paths_(),
162162
end_paths_(),
@@ -245,7 +245,6 @@ namespace edm {
245245
}
246246
//determine if this module could read a branch we want to delete early
247247
auto consumes = modReg.getExistingModule(w->description()->moduleLabel())->moduleConsumesInfos();
248-
//auto consumes = w->moduleConsumesInfos();
249248
if (not consumes.empty()) {
250249
bool foundAtLeastOneMatchingBranch = false;
251250
for (auto const& product : consumes) {

0 commit comments

Comments
 (0)