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.

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 FWLite #13200

Merged
merged 1 commit into from Feb 16, 2016

Conversation

wmtan
Copy link
Contributor

@wmtan wmtan commented Feb 5, 2016

This PR partially implements deep const correctness in FWLite by using the propagate_const template around most pointers (bare or smart) that are data members of classes or structs, and making the other changes required to do this. Pointers to const and pointers declared mutable do not need propagate_const. A very few pointers were left undone because the changes involved would be complex enough that more attention is required.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

A new Pull Request was created by @wmtan for CMSSW_8_0_X.

It involves the following packages:

DataFormats/FWLite
FWCore/FWLite
FWCore/TFWLiteSelector
FWCore/TFWLiteSelectorTest

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

cms-bot commands are list here #13028

@Dr15Jones
Copy link
Contributor

Please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

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

getter_ = getter;
}

edm::EDProductGetter* getter() {
edm::EDProductGetter const* getter() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the function have become const as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should be const. Will be fixed.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

There was an issue with git-cms-merge-topic you can see the log here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13200/11017/git-merge-result

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

Pull request #13200 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please check and sign again.

@wmtan
Copy link
Contributor Author

wmtan commented Feb 5, 2016

@Dr15Jones Updated to incorporate both of your comments.

@wmtan
Copy link
Contributor Author

wmtan commented Feb 5, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

-1
Tested at: 4f9af0f
I found an error when building:

Error in UnknownClass::WorkingDirectory: getcwd() failed
Entering library rule at src/DQMServices/Components/plugins
>> Compiling LCG dictionary: tmp/slc6_amd64_gcc493/src/DataFormats/FWLite/src/DataFormatsFWLite/a/DataFormatsFWLite_xr.cc
Error in UnknownClass::WorkingDirectory: getcwd() failed
>> Compiling LCG dictionary: tmp/slc6_amd64_gcc493/src/FWCore/FWLite/src/FWCoreFWLite/a/FWCoreFWLite_xr.cc
cc1plus: error: DataFormatsFWLite/a/DataFormatsFWLite_xr.cc: Bad address
/bin/sh: line 1: 20502 Segmentation fault      /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/lcg/root/6.06.00-agmdcp/bin/genreflex src/PhysicsTools/ParallelAnalysis/src/classes.h -s src/PhysicsTools/ParallelAnalysis/src/classes_def.xml -o tmp/slc6_amd64_gcc493/src/PhysicsTools/ParallelAnalysis/src/PhysicsToolsParallelAnalysis/a/PhysicsToolsParallelAnalysis_xr.cc --deep --fail_on_warnings --rootmap=PhysicsToolsParallelAnalysis_xr.rootmap --rootmap-lib=libPhysicsToolsParallelAnalysis.so -m FWCoreTFWLiteSelector_xr_rdict.pcm -m FWCoreCommon_xr_rdict.pcm -m DataFormatsCommon_xr_rdict.pcm -m DataFormatsProvenance_xr_rdict.pcm -m FWCoreMessageLogger_xr_rdict.pcm -m DataFormatsStdDictionaries_xr_rdict.pcm -m DataFormatsStdDictionaries_x1r_rdict.pcm -m DataFormatsStdDictionaries_x2r_rdict.pcm -m DataFormatsStdDictionaries_x3r_rdict.pcm --capabilities=PhysicsToolsParallelAnalysis_xi.cc -DCMS_DICT_IMPL -D_REENTRANT -DGNUSOURCE -D__STRICT_ANSI__ -DGNU_GCC -D_GNU_SOURCE -DCMSSW_GIT_HASH='"CMSSW_8_0_X_2016-02-04-2300"' -DPROJECT_NAME='"CMSSW"' -DPROJECT_VERSION='"CMSSW_8_0_X_2016-02-04-2300"' -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -I/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-02-04-2300/src -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-02-04-2300/src -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/lcg/root/6.06.00-agmdcp/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/boost/1.57.0-kpegke/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/pcre/7.9__8.33/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/bz2lib/1.0.5/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/clhep/2.2.0.4-kpegke/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/libuuid/2.22.2/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/python/2.7.6-kpegke/include/python2.7 -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/tbb/44_20151115oss/include -I/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/zlib/1.2.8/include -DCMSSW_REFLEX_DICT
gmake: **\* [tmp/slc6_amd64_gcc493/src/PhysicsTools/ParallelAnalysis/src/PhysicsToolsParallelAnalysis/a/PhysicsToolsParallelAnalysis_xr.cc] Error 139
>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-02-04-2300/src/DataFormats/FWLite/test/TestRunnerDataFormatsFWLite.cpp 
gmake: **\* [tmp/slc6_amd64_gcc493/src/DataFormats/FWLite/src/DataFormatsFWLite/a/DataFormatsFWLite_xr.o] Error 1
gmake: **\* [tmp/slc6_amd64_gcc493/src/DataFormats/FWLite/test/testDataFormatsFWLite/test.cppunit.o] Segmentation fault


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

@wmtan
Copy link
Contributor Author

wmtan commented Feb 5, 2016

The error is not caused by this PR.

@wmtan
Copy link
Contributor Author

wmtan commented Feb 5, 2016

@cmsbuild please test


/**Returns true if a record with the name iRecordName is available in the file
*/
bool exists(const char* iRecordName) const;
bool exists(const char* iRecordName);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can now be const again.

@Dr15Jones
Copy link
Contributor

I've finished my second review

@cmsbuild
Copy link
Contributor

Pull request #13200 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please check and sign again.

@wmtan
Copy link
Contributor Author

wmtan commented Feb 15, 2016

@Dr15Jones Updated to incorporate all three of your latest round of comments.

@wmtan
Copy link
Contributor Author

wmtan commented Feb 15, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

davidlange6 added a commit that referenced this pull request Feb 16, 2016
Implement propagate_const in FWLite
@davidlange6 davidlange6 merged commit c4f92f5 into cms-sw:CMSSW_8_0_X Feb 16, 2016
@wmtan wmtan deleted the PropagateConstInFWLite branch February 16, 2016 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants