Silence static analyzer warning on existsAs() calls in PluginDescription.h #42889

makortel · 2023-09-27T14:20:17Z

PR description:

As far as I can (quickly) tell, these two uses of existsAs() are legitimate. The warnings were noticed in #42518 (comment) .

Resolves cms-sw/framework-team#681

PR validation:

None (edited on the web editor).

cmsbuild · 2023-09-27T14:25:55Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42889/37024

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42889/37024/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42889/37024/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

…ion.h These two uses of existsAs() are legitimate

cmsbuild · 2023-09-27T14:43:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42889/37025

This PR adds an extra 16KB to repository

cmsbuild · 2023-09-27T14:43:54Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

FWCore/ParameterSet (core)

@Dr15Jones, @smuzaffar, @makortel, @cmsbuild can you please review it and eventually sign? Thanks.
@missirol, @wddgit this is something you requested to watch as well.
@antoniovilela, @sextonkennedy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

makortel · 2023-09-27T14:53:31Z

@cmsbuild, please test

Let's see if this works

makortel · 2023-09-27T14:53:41Z

@Dr15Jones Do you agree?

Dr15Jones · 2023-09-27T14:56:01Z

yes

cmsbuild · 2023-09-27T17:30:11Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-55493a/34942/summary.html
COMMIT: 0b23d79
CMSSW: CMSSW_13_3_X_2023-09-27-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42889/34942/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-55493a/34942/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-55493a/34942/git-merge-result

Unit Tests

I found 1 errors in the following unit tests:

---> test CondToolsLHCInfoNewPopConTest had ERRORS

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3358320
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3358292
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2023-09-27T18:28:53Z

Seems to do the job. The unit test fails also in the IBs

makortel · 2023-09-27T18:28:56Z

+core

cmsbuild · 2023-09-27T18:29:11Z

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

srimanob · 2023-09-28T08:40:31Z

@cmsbuild please test with #42518

Just to see the result.

cmsbuild · 2023-09-28T16:57:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-55493a/34960/summary.html
COMMIT: 0b23d79
CMSSW: CMSSW_13_3_X_2023-09-28-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42889/34960/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 35 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3358320
DQMHistoTests: Total failures: 133
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3358165
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

srimanob · 2023-09-28T18:20:52Z

Hi @makortel

Thanks very much for the PR. I still see warnings of existAs() from the last test that I trigger in the PR, PR test result. Do I miss something? Thx.

In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-09-28-1100/src/RecoHGCal/TICL/plugins/TrackstersMergeProducer.cc:5:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-09-28-1100/src/FWCore/ParameterSet/interface/PluginDescription.h:193:27: warning: function edm::ParameterSet::existsAs<class std::basic_string<char>>(const std::basic_string<char, std::char_traits<char>, std::allocator<char> > &, _Bool) const is called in function edm::PluginDescription::exists_(const edm::ParameterSet &) const [cms.CodeRules.psetExistsChecker]
      CMS_SA_ALLOW return pset.existsAs<std::string>(typeLabel_, typeLabelIsTracked_);
                          ^    ~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-09-28-1100/src/FWCore/ParameterSet/interface/PluginDescription.h:193:27: warning: function edm::ParameterSet::existsAs<class std::basic_string<char>>(const std::basic_string<char, std::char_traits<char>, std::allocator<char> > &, _Bool) const is called in function edm::PluginDescription<edmplugin::PluginFactory<ticl::LinkingAlgoBase *(const edm::ParameterSet &)>>::exists_(const edm::ParameterSet &) const [cms.CodeRules.psetExistsChecker]
      CMS_SA_ALLOW return pset.existsAs<std::string>(typeLabel_, typeLabelIsTracked_);
                          ^    ~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-09-28-1100/src/FWCore/ParameterSet/interface/PluginDescription.h:203:26: warning: function edm::ParameterSet::existsAs<class std::basic_string<char>>(const std::basic_string<char, std::char_traits<char>, std::allocator<char> > &, _Bool) const is called in function edm::PluginDescription::findType(const edm::ParameterSet &) const [cms.CodeRules.psetExistsChecker]
        CMS_SA_ALLOW if (iPSet.existsAs<std::string>(typeLabel_) || defaultType_.empty()) {
                         ^     ~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-09-28-1100/src/FWCore/ParameterSet/interface/PluginDescription.h:203:26: warning: function edm::ParameterSet::existsAs<class std::basic_string<char>>(const std::basic_string<char, std::char_traits<char>, std::allocator<char> > &, _Bool) const is called in function edm::PluginDescription<edmplugin::PluginFactory<ticl::LinkingAlgoBase *(const edm::ParameterSet &)>>::findType(const edm::ParameterSet &) const [cms.CodeRules.psetExistsChecker]
        CMS_SA_ALLOW if (iPSet.existsAs<std::string>(typeLabel_) || defaultType_.empty()) {

makortel · 2023-09-28T18:34:30Z

I'm honestly puzzled why the warnings appear only in the tests with #42518?

rappoccio · 2023-09-29T13:25:05Z

Ready to merge except we also need this one:

#42518

makortel · 2023-09-29T13:29:54Z

This PR does not need #42518, @srimanob just wanted to test them together.

srimanob · 2023-09-29T15:04:38Z

@cms-sw/orp-l2 please feel free to merge. I will follow up on remaining static log warning in the original PR. Thx.

FWCore/ParameterSet/interface/PluginDescription.h

rappoccio · 2023-10-02T13:42:30Z

+1

cmsbuild added this to the CMSSW_13_3_X milestone Sep 27, 2023

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Sep 27, 2023

makortel mentioned this pull request Sep 27, 2023

Silence static analyzer warning on existsAs() calls in PluginDescription.h cms-sw/framework-team#681

Closed

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 27, 2023

Silence static analyzer warning on existsAs() calls in PluginDescript…

0b23d79

…ion.h These two uses of existsAs() are legitimate

makortel force-pushed the patch-6 branch from 6f26ee9 to 0b23d79 Compare September 27, 2023 14:37

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 27, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 27, 2023

cmsbuild added tests-started and removed tests-pending labels Sep 27, 2023

cmsbuild added tests-rejected and removed tests-started labels Sep 27, 2023

cmsbuild removed core-pending pending-signatures labels Sep 27, 2023

cmsbuild added core-approved fully-signed labels Sep 27, 2023

cmsbuild added requires-external tests-started and removed tests-rejected labels Sep 28, 2023

cmsbuild added tests-approved and removed tests-started labels Sep 28, 2023

rappoccio mentioned this pull request Sep 29, 2023

Improve MC truth information in TICL, Sim-Reco Associators, TICLDumper #42518

Merged

srimanob reviewed Sep 29, 2023

View reviewed changes

FWCore/ParameterSet/interface/PluginDescription.h Show resolved Hide resolved

cmsbuild added orp-approved and removed orp-pending labels Oct 2, 2023

cmsbuild merged commit 4586ae0 into cms-sw:master Oct 2, 2023

makortel deleted the patch-6 branch October 2, 2023 13:56

This was referenced Oct 3, 2023

New schema evolution unit test #42825

Merged

Updated root to tip of branch master cms-sw/cmsdist#8734

Closed

missirol mentioned this pull request Jan 2, 2024

understanding how to use (and test) CMS_SA_ALLOW #43646

Closed

Silence static analyzer warning on existsAs() calls in PluginDescription.h #42889

Silence static analyzer warning on existsAs() calls in PluginDescription.h #42889

Uh oh!

Conversation

makortel commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented Sep 27, 2023

Uh oh!

cmsbuild commented Sep 27, 2023

Uh oh!

cmsbuild commented Sep 27, 2023

Uh oh!

makortel commented Sep 27, 2023

Uh oh!

makortel commented Sep 27, 2023

Uh oh!

Dr15Jones commented Sep 27, 2023

Uh oh!

cmsbuild commented Sep 27, 2023

Unit Tests

Comparison Summary

Uh oh!

makortel commented Sep 27, 2023

Uh oh!

makortel commented Sep 27, 2023

Uh oh!

cmsbuild commented Sep 27, 2023

Uh oh!

srimanob commented Sep 28, 2023

Uh oh!

cmsbuild commented Sep 28, 2023

Comparison Summary

Uh oh!

srimanob commented Sep 28, 2023

Uh oh!

makortel commented Sep 28, 2023

Uh oh!

rappoccio commented Sep 29, 2023

Uh oh!

makortel commented Sep 29, 2023

Uh oh!

srimanob commented Sep 29, 2023

Uh oh!

Uh oh!

rappoccio commented Oct 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

makortel commented Sep 27, 2023 •

edited

Loading