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

propagate const for pointer data members #13015

Closed
wants to merge 1 commit into from

Conversation

wmtan
Copy link
Contributor

@wmtan wmtan commented Jan 20, 2016

This PR uses the propagate_const template for most pointers in the framework that are data members of classes or structs, and makes the knock on changes caused by this. This means that a const member function of the class will no longer be able to modify the data to which the pointer points. This PR is therefore a partial implementation of issue #12840. The member pointers in the framework that are not modified by this PR are:

  1. Those in FWLite (will be done in a separate PR).
  2. Pointers to const or pointers already declared mutable (no need to modify).
  3. Those pointers whose modifications require changes that were too extensive or complex to be done in this PR. These will be dealt within a case by case basis.
    Other minor changes made in a very few places in affected code:
    a) a few boost::scoped_ptr changed to std::unique_ptr
    b) a few std::auto_ptr changed to std::unique_ptr
    c) iterator loops replaced by range based for loops
    d) nullptr replaces 0 for pointers

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DataFormats/Common
DataFormats/Provenance
FWCore/Catalog
FWCore/Framework
FWCore/Integration
FWCore/MessageLogger
FWCore/MessageService
FWCore/Modules
FWCore/ParameterSet
FWCore/PluginManager
FWCore/ServiceRegistry
FWCore/Services
FWCore/Sources
FWCore/Utilities
IOMC/Input
IOMC/RandomEngine
IOPool/Common
IOPool/Input
IOPool/Output
IOPool/SecondaryInput
IOPool/Streamer
IOPool/TFileAdaptor
Utilities/StorageFactory
Utilities/XrdAdaptor

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

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@Dr15Jones
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor


private:
T* ptr_;
edm::propagate_const<T*> ptr_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this cause ROOT to generate an extra layer in the storage?

@cmsbuild
Copy link
Contributor

-1
Tested at: 8abafcd
I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 8 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-20-2300/src/FWCore/Utilities/src/DebugMacros.cc 
>> Compiling  /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-20-2300/src/FWCore/Utilities/src/DictionaryTools.cc 
In file included from /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-20-2300/src/FWCore/PluginManager/src/PluginManager.cc:25:
In file included from /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-20-2300/src/FWCore/PluginManager/interface/PluginManager.h:29:
In file included from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/tbb/44_20150728oss/include/tbb/concurrent_unordered_map.h:27:
/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/tbb/44_20150728oss/include/tbb/internal/_concurrent_unordered_impl.h:277:57: error: call to implicitly-deleted copy constructor of 'std::pair > >'
            new(static_cast(&pnode->my_element)) T(tbb::internal::forward(t));
                                                        ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc493/external/tbb/44_20150728oss/include/tbb/internal/_concurrent_unordered_impl.h:1293:40: note: in instantiation of function template specialization 'tbb::interface5::internal::split_ordered_list > >, tbb::tbb_allocator > > > >::create_node > > >' requested here
                     pnode = my_solist.create_node(order_key, tbb::internal::forward(value));
                                       ^


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

@cmsbuild
Copy link
Contributor

@wmtan
Copy link
Contributor Author

wmtan commented Jan 27, 2016

@Dr15Jones This PR is superseded by #13093, so I am closing it.

@wmtan wmtan closed this Jan 27, 2016
@wmtan wmtan deleted the PropagateConst branch February 5, 2016 20:20
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

3 participants