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.
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
Fix ODR problems and includes in Selections.h #19353
Conversation
Part of the work going on regarding the C++ modules migration of CMSSW (tracked as issue #15248). This PR is not meant to refactor things, but just to make these headers compile. |
A new Pull Request was created by @Teemperor (Raphael Isemann) for master. It involves the following packages: PhysicsTools/UtilAlgos @cmsbuild, @monttj, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
2415338
to
3d88305
Compare
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 3d88305 You can see the results of the tests here: I found follow errors while testing this PR Failed tests: Build ClangBuild
I found an error when building: >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/plugins/ConfigurableAnalysis.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/plugins/NTuplingDevice.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/plugins/PrimaryVertexFilter.cc In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/plugins/ConfigurableAnalysis.cc:36:0: /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/interface/Selections.h: In constructor 'FilterSelections::FilterSelections(const edm::ParameterSet&, edm::ConsumesCollector&&)': /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/interface/Selections.h:389:22: error: 'Selection' was not declared in this scope for (std::vector::iterator sIt=selections_.begin();sIt!=selections_.end();++sIt){ ^ /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/interface/Selections.h:389:31: error: template argument 1 is invalid for (std::vector::iterator sIt=selections_.begin();sIt!=selections_.end();++sIt){ ^
I found a compilation error while trying to compile with clang: >> Compiling /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/src/BasicMuonAnalyzer.cc >> Compiling /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/src/StringBasedNTupler.cc >> Compiling /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/bin/FWLiteWithBasicAnalyzer.cc >> Compiling /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/bin/mergeTFileServiceHistograms.cpp In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/plugins/ConfigurableAnalysis.cc:36: /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/interface/Selections.h:389:22: error: use of undeclared identifier 'Selection' for (std::vector::iterator sIt=selections_.begin();sIt!=selections_.end();++sIt){ ^ /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_2_X_2017-06-19-1100/src/PhysicsTools/UtilAlgos/interface/Selections.h:389:32: error: no type named 'iterator' in the global namespace; did you mean simply 'iterator'? for (std::vector::iterator sIt=selections_.begin();sIt!=selections_.end();++sIt){ ^~~~~~~~~~ |
Comparison not run due to Build errors (RelVals and Igprof tests were also skipped) |
We include MessageLogger.h because we use edm::LogVerbatim in this header and this way it can compile on its own. We renamed Selection to FilterSelection because we already have a Selection class in CommonTools/Utils/interface/Selection.h, which is breaking the One Definition Rule and breaks the C++ modules builds (and can also break the normal CMSSW builds).
3d88305
to
52c8d85
Compare
Pull request #19353 was updated. @cmsbuild, @monttj, @davidlange6 can you please check and sign again. |
Fixed compilation errors. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
merge |
We include MessageLogger.h because we use edm::LogVerbatim in this
header and this way it can compile on its own.
We renamed Selection to FilterSelection because we already have
a Selection class in CommonTools/Utils/interface/Selection.h,
which is breaking the One Definition Rule and breaks the C++
modules builds (and can also break the normal CMSSW builds).