Commonality with ROOT6 version in CommonTools/Utils #8534
Conversation
|
A new Pull Request was created by @wmtan for CMSSW_7_4_X. Commonality with ROOT6 version in CommonTools/Utils It involves the following packages: CommonTools/Utils @nclopezo, @cvuosalo, @monttj, @cmsbuild, @slava77, @vadler can you please review it and eventually sign? Thanks. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
| ExpressionVar::makeStorage(edm::ObjectWithDict& obj, | ||
| const edm::TypeWithDict& retType) | ||
| { | ||
| static edm::TypeWithDict tVoid(edm::TypeWithDict::byName("void")); |
There was a problem hiding this comment.
tVoid should be made const for thread safety. If you have to change this PR for other reasons, it would be good to fix tVoid at the same time. Otherwise, it can wait until later.
There was a problem hiding this comment.
tVoid cannot be declared const because it is a static function. There is no object to be const in a static function.
There was a problem hiding this comment.
Unless I'm missing something, tVoid is an instance of the class edm::TypeWithDict. It is not a function. It is declared static so that it will be initialized only once and will preserve its value thereafter. It could probably be made static const without causing problems.
But it does not need to be fixed in this PR, unless other changes are also being made.
There was a problem hiding this comment.
You are correct. Yes, it probably should be declared const.
|
+1 Re-formatting of code for ROOT5/ROOT6 compatibility. There should be no change in functionality. Jenkins tests against baseline CMSSW_7_4_X_2015-03-24-2300 are OK and show no significant differences, as expected. The matching PR #8534 for the ROOT5 branch has a pre-existing compile error in that branch, but this PR does not seem to have any compile problems of its own, even though GitHub reports such an error. |
Commonality with ROOT6 version in CommonTools/Utils
This pull request is for commonality in the CMSSW code for the package CommonTools/Utils between CMSSW_7_4_ROOT6_X and CMSSW_7_4_X. All the changes in this package in CMSSW_7_4_ROOT6_X that are compatible with ROOT5 are ported back in this PR. The changes may seem extensive, but much of it is white space changes and reformatting.
The CondCore/Utils unit tests and the entire relval matrix pass with this PR.