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

CondFormats/JetMETObjects : Fix clang static analyzer warnings #16283

Merged
merged 4 commits into from Oct 24, 2016
Merged

CondFormats/JetMETObjects : Fix clang static analyzer warnings #16283

merged 4 commits into from Oct 24, 2016

Conversation

gartung
Copy link
Member

@gartung gartung commented Oct 20, 2016

Data Class Const Correctness Const function returns pointer or reference to non-const member data object /CondFormats/JetMETObjects/interface/JetResolutionObject.h unknown 190
Data Class Const Correctness Const function returns pointer or reference to non-const object /CondFormats/JetMETObjects/interface/JetResolutionObject.h getFormula 189

Data Class Const Correctness	Const function returns pointer or reference to non-const member data object	/CondFormats/JetMETObjects/interface/JetResolutionObject.h	unknown	190
Data Class Const Correctness	Const function returns pointer or reference to non-const object	/CondFormats/JetMETObjects/interface/JetResolutionObject.h	getFormula	189
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gartung (Patrick Gartung) for CMSSW_8_1_X.

It involves the following packages:

CondFormats/JetMETObjects

@ggovi, @cmsbuild, @monttj, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @ahinzmann, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gkasieczka, @apfeiffer1, @schoef, @mariadalfonso 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

@gartung
Copy link
Member Author

gartung commented Oct 20, 2016

@Dr15Jones

@Dr15Jones
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 20, 2016

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

@cmsbuild
Copy link
Contributor

-1

Tested at: bcdb9f9

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
0eb854c
9acf42d
6a16a5e
7ad8098
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16283/15857/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16283/15857/git-merge-result

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

I found follow errors while testing this PR

Failed tests: Build ClangBuild

  • Build:

I found an error when building:

>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/QGLikelihoodObject.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/JetResolution.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/JetCorrectorParameters.cc 
>> Compiling  tmp/slc6_amd64_gcc530/src/CondFormats/JetMETObjects/src/CondFormatsJetMETObjects/a/Serialization.cc 
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/JetResolutionObject.cc: In member function 'float JME::JetResolutionObject::evaluateFormula(const JME::JetResolutionObject::Record&, const JME::JetParameters&) const':
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/JetResolutionObject.cc:388:52: error: invalid conversion from 'const TFormula_' to 'TFormula_' [-fpermissive]
         TFormula\* formula = m_definition.getFormula();
                                                    ^
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/Geometry/HGCalCommonData/src/HGCalGeometryMode.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/Geometry/HGCalCommonData/src/HGCalParameters.cc 
Entering library rule at Geometry/CaloTopology

  • Clang:

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

>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/TestGetterOfProducts.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/ValueExamplePlugin.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/Doodad.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/DoodadESSource.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/GadgetRcd.cc 
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/CondFormats/JetMETObjects/src/JetResolutionObject.cc:388:19: error: cannot initialize a variable of type 'TFormula _' with an rvalue of type 'const TFormula *'
        TFormula_ formula = m_definition.getFormula();
                  ^         ~~~~~~~~~~~~~~~~~~~~~~~~~
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/ProdigalAnalyzer.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-20-1100/src/FWCore/Integration/test/ValueExample.cc 
1 error generated.


The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
0eb854c
9acf42d
6a16a5e
7ad8098
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16283/15857/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16283/15857/git-merge-result

@cmsbuild
Copy link
Contributor

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

@gartung
Copy link
Member Author

gartung commented Oct 20, 2016

Doh. I forgot to compiler after fixing the static analyzer warning.

@cmsbuild
Copy link
Contributor

Pull request #16283 was updated. @ggovi, @cmsbuild, @monttj, @davidlange6 can you please check and sign again.

@gartung
Copy link
Member Author

gartung commented Oct 20, 2016

@Dr15Jones

@@ -385,7 +385,7 @@ namespace JME {
return 1;

// Set parameters
TFormula* formula = m_definition.getFormula();
TFormula* formula = new TFormula( * (m_definition.getFormula() ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

better to do

auto const* pFormula = m_definition.getFormula();
if( ! formula) { return 1;}

return TFormula(*pFormula)->EvalPar(variables_);

Copy link
Member Author

Choose a reason for hiding this comment

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

Not quite. The formula is modified with new parameters.

@cmsbuild
Copy link
Contributor

Pull request #16283 was updated. @ggovi, @cmsbuild, @monttj, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #16283 was updated. @ggovi, @cmsbuild, @monttj, @davidlange6 can you please check and sign again.

@Dr15Jones
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 20, 2016

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

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@monttj
Copy link
Contributor

monttj commented Oct 24, 2016

+1

@davidlange6 davidlange6 merged commit 938df5e into cms-sw:CMSSW_8_1_X Oct 24, 2016
@gartung gartung deleted the CondFormats-JetMETObjects-clangSA-warning-fix branch December 1, 2016 02:21
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

5 participants