Fix clang compiler warning and replace boost::mpl::if_c with std::conditional #16821

knoepfel · 2016-11-30T16:10:02Z

Fixed some clang compiler warnings.
Also replaced several uses of boost::mpl::if_c with std::conditional.

Some white space changes, too.

cmsbuild · 2016-11-30T16:10:24Z

A new Pull Request was created by @knoepfel (Kyle Knoepfel) for CMSSW_9_0_X.

It involves the following packages:

DataFormats/FWLite
FWCore/Framework
FWCore/ParameterSet
FWCore/Services
IOPool/Input
IOPool/Output
IOPool/Streamer

@cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit, @wmtan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

knoepfel · 2016-11-30T16:17:10Z

Sorry, looks like a lot of white-space-only changes in this request.

Dr15Jones · 2016-11-30T16:18:15Z

please test

Dr15Jones · 2016-11-30T16:18:20Z

+1

cmsbuild · 2016-11-30T16:18:39Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16688/console

cmsbuild · 2016-11-30T16:18:40Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

cmsbuild · 2016-11-30T19:55:33Z

-1

Tested at: 49a7908

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
03fb439
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16688/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16688/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16688/summary.html

I found follow errors while testing this PR

Failed tests: Build

Build:

I found an error when building:

>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/CaloOnlineTools/EcalTools/plugins/EcalExclusiveTrigFilter.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/CaloOnlineTools/EcalTools/plugins/EcalPnGraphs.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/CaloOnlineTools/EcalTools/plugins/EcalMipGraphs.cc 
In file included from input_line_9:51:
In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/DPGAnalysis/SiStripTools/interface/Multiplicities.h:6:
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/FWCore/Framework/interface/Event.h:373:10: error: no member named 'conditional_t' in namespace 'std'
    std::conditional_t::value,
    ~~~~~^
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/FWCore/Framework/interface/Event.h:384:10: error: no member named 'conditional_t' in namespace 'std'
    std::conditional_t::value,
    ~~~~~^

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
03fb439
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16688/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16688/git-merge-result

cmsbuild · 2016-11-30T19:55:36Z

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

knoepfel · 2016-11-30T20:10:08Z

What's going on here? The std::conditional_t typedef was introduced in C++14. To use it #include <type_traits> must be included. To my knowledge, the code was being compiled with the c++14 flag, and I have included the type-traits header.

Dr15Jones · 2016-11-30T20:26:37Z

Looking at the full build log, I believe this is a rootcling problem. Clearing out the overlapping output caused by multiple processes used in the compilation leaves

>> Building LCG reflex dict from header file src/DPGAnalysis/SiStripTools/src/classes.h
In file included from input_line_9:51:
In file included from /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/DPGAnalysis/SiStripTools/interface/Multiplicities.h:6:
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/FWCore/Framework/interface/Event.h:373:10: error: no member named 'conditional_t' in namespace 'std'
    std::conditional_t<detail::has_postinsert<PROD>::value,
    ~~~~~^
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/FWCore/Framework/interface/Event.h:384:10: error: no member named 'conditional_t' in namespace 'std'
    std::conditional_t<detail::has_donotrecordparents<PROD>::value,
    ~~~~~^
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_0_X_2016-11-30-1100/src/FWCore/Framework/interface/Event.h:387:5: error: use of undeclared identifier 'parentage_recorder'
    parentage_recorder.do_it(putProducts(),
    ^
Error: rootcling: compilation failure (tmp/slc6_amd64_gcc530/src/DPGAnalysis/SiStripTools/src/DPGAnalysisSiStripTools/a/DPGAnalysisSiStripTools_xr73ad0eea2a_dictUmbrella.h)
gmake: *** [tmp/slc6_amd64_gcc530/src/DPGAnalysis/SiStripTools/src/DPGAnalysisSiStripTools/a/DPGAnalysisSiStripTools_xr.cc] Error 1

@smuzaffar do we have all the proper flags passed to cling?
@pcanal is this a known problem with ROOT 6.06?

Dr15Jones · 2016-11-30T20:40:01Z

The head in question
http://cmslxr.fnal.gov/source/DPGAnalysis/SiStripTools/interface/Multiplicities.h

violates CMS policy that DataFormats should be seperate from the algorithm which produces them. Instead, the classes in this header attempt to create themselves (and therefore are trying to access edm::Event which no data product should do).

Dr15Jones · 2016-11-30T20:51:39Z

The only file that actually uses Multiplicities.h is
http://cmslxr.fnal.gov/source/DPGAnalysis/SiStripTools/plugins/ByMultiplicityEventFilter.cc

and the classes in question are used as helpers and NOT as data formats. Therefore the Wrapper declarations are unnecessary.

However, that class does want dictionaries generated since it uses those classes with the StringCutObjectSelector so they can do simple comparisons from the configuration!

Dr15Jones · 2016-12-03T14:40:34Z

please test

cmsbuild · 2016-12-03T14:40:52Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16751/console

Dr15Jones · 2016-12-03T14:45:08Z

ROOT was rebuilt in the most recent IB to use C++14. This will be used in the next test.

Dr15Jones · 2016-12-03T14:57:35Z

@smuzaffar looks like the ROOT change broke stuff but we missed it in the IB since we didn't do a full rebuild of CMSSW after the change.

Dr15Jones · 2016-12-03T15:03:52Z

I took a look at the failing file, but can't see any reason for the missing class error since the file does include the header file which declares that class. In addition, no ROOT specific defines are in any of those headers.
Maybe it was a transient file reading glitch?

smuzaffar · 2016-12-03T15:44:45Z

@Dr15Jones , CMSSW_9_0_X_2016-12-02-2300 was a full build with root updates.

cmsbuild · 2016-12-03T18:59:37Z

-1

Tested at: a17221c

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16751/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

Unit Tests:

I found errors in the following unit tests:

---> test testRecoMETMETProducers had ERRORS

cmsbuild · 2016-12-03T18:59:39Z

Comparison job queued.

Dr15Jones · 2016-12-03T20:19:31Z

+1

Dr15Jones · 2016-12-03T20:20:16Z

The unit test failure has nothing to do with this pull request. (The test failed because the file generated for the test could not be found, which probably means a race condition in the testing infrastructure.)

cmsbuild · 2016-12-03T20:37:38Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16821/16751/summary.html

The workflows 1003.0, 1001.0, 1000.0, 140.53, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

davidlange6 · 2016-12-05T16:41:10Z

hi @knoepfel - could you edit the pull request title to be more reflective of changes made for the release notes. Thanks.

knoepfel · 2016-12-05T16:43:50Z

@davidlange6 done.

knoepfel added 2 commits November 30, 2016 09:24

Resolve some clang warnings.

e744cdb

Replace some uses of boost::mpl::if_c with std::conditional.

49a7908

Some white space changes, too.

cmsbuild added this to the Next CMSSW_9_0_X milestone Nov 30, 2016

cmsbuild added comparison-pending core-pending orp-pending pending-signatures tests-pending labels Nov 30, 2016

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Nov 30, 2016

cmsbuild added tests-started and removed tests-pending labels Nov 30, 2016

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Nov 30, 2016

cmsbuild added the tests-rejected label Dec 3, 2016

cmsbuild added comparison-pending tests-pending tests-started and removed comparison-notrun tests-rejected tests-pending labels Dec 3, 2016

cmsbuild added tests-rejected and removed tests-started labels Dec 3, 2016

cmsbuild added core-approved and removed core-pending labels Dec 3, 2016

cmsbuild added comparison-available and removed comparison-pending labels Dec 3, 2016

knoepfel changed the title ~~General cleanup~~ Fix clang compiler warning and replace boost::mpl::if_c with std::conditional Dec 5, 2016

davidlange6 merged commit e19d1b1 into cms-sw:CMSSW_9_0_X Dec 5, 2016

knoepfel deleted the cleanup branch December 6, 2016 16:27

slava77 mentioned this pull request Aug 3, 2017

attempt to fix SiStrip DQM reproducibility issue: allign members common to root and full c++ #20050

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix clang compiler warning and replace boost::mpl::if_c with std::conditional #16821

Fix clang compiler warning and replace boost::mpl::if_c with std::conditional #16821

knoepfel commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

knoepfel commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

cmsbuild commented Nov 30, 2016 •

edited

cmsbuild commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

knoepfel commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Dec 3, 2016

cmsbuild commented Dec 3, 2016 •

edited

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

smuzaffar commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

davidlange6 commented Dec 5, 2016

knoepfel commented Dec 5, 2016

Fix clang compiler warning and replace boost::mpl::if_c with std::conditional #16821

Fix clang compiler warning and replace boost::mpl::if_c with std::conditional #16821

Conversation

knoepfel commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

knoepfel commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

cmsbuild commented Nov 30, 2016 • edited

cmsbuild commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

cmsbuild commented Nov 30, 2016

knoepfel commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Nov 30, 2016

Dr15Jones commented Dec 3, 2016

cmsbuild commented Dec 3, 2016 • edited

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

smuzaffar commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

Dr15Jones commented Dec 3, 2016

cmsbuild commented Dec 3, 2016

davidlange6 commented Dec 5, 2016

knoepfel commented Dec 5, 2016

cmsbuild commented Nov 30, 2016 •

edited

cmsbuild commented Dec 3, 2016 •

edited