Add edm::Principal::getByToken to skipped functions regex

[gartung]

Hi Patrick,
	The algorithm we are using to do the static analysis checks for modules is generally very helpful. I have found a case where it is giving a false positive and was wondering if you could update the code to avoid that case? An example is

In call stack '  CompareGeneratorResultsAnalyzer::globalBeginLuminosityBlock() const calls function  edm::LuminosityBlock::get<class GenLumiInfoHeader>() const calls function  edm::PrincipalGetAdapter::getByToken_() const calls function  edm::Principal::getByToken() const calls function  edm::ProductResolverBase::resolveProduct() const calls function  edm::ProductResolverBase::resolveProduct_() const virtual calls function  edm::UnscheduledProductResolver::resolveProduct_() const calls function  edm::Worker::doWork<class edm::OccurrenceTraits<class edm::EventPrincipal, BranchActionStreamBegin>>() calls function  edm::Worker::runModule<class edm::OccurrenceTraits<class edm::EventPrincipal, BranchActionStreamBegin>>() calls function  edm::workerhelper::CallImpl<edm::OccurrenceTraits<edm::EventPrincipal, BranchActionStreamBegin> >::call() calls function  edm::Worker::implDo() virtual calls function  edm::WorkerT<edm::one::OutputModuleBase>::implDo() calls function  edm::one::OutputModuleBase::doEvent() calls function  edm::one::OutputModuleBase::write() virtual calls function  RawEventOutputModuleForBU::write() calls function  crc32c() calls function  crc32c_hw() static variable  crc32c_long' is accessed , 'CompareGeneratorResultsAnalyzer::globalBeginLuminosityBlock() const' overrides 'edm::global::impl::LuminosityBlockCacheHolder<edm::global::EDAnalyzerBase, cgra::DummyCache>::globalBeginLuminosityBlock() const virtual'

The false positive comes from the chain where a particular virtual function could theoretically be called, but in reality never is:

edm::PrincipalGetAdapter::getByToken_() const calls function  edm::Principal::getByToken() const calls function  edm::ProductResolverBase::resolveProduct() const calls function  edm::ProductResolverBase::resolveProduct_() const virtual calls function  edm::UnscheduledProductResolver::resolveProduct_() ...

My suggestion is to filter out any call chain containing edm::Principal::getByToken().

	Thanks,
		Chris

[gartung]

@makortel @Dr15Jones

[cmsbuild]

The code-checks are being triggered in jenkins.

[Dr15Jones]

@gartung thanks!

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30146/15930

• This PR adds an extra 12KB to repository

[cmsbuild]

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

It involves the following packages:

Utilities/StaticAnalyzers

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

cms-bot commands are listed here

[makortel]

@cmsbuild, please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6879/console Started: 2020/06/08 16:26

[makortel]

I find the long line of ors difficult to digest. I wonder if e.g. something along

skipfunc = re.compile("|".join([
    "(edm::(LuminosityBlock::|Run::|Event::|Principal::)getBy(Label|Token))",
    ...
]))

would make it easier to read?

[gartung]

My initial attempts at this produces a larger list in statics2modules.txt and modules2statics.txt

[makortel]

Ok. I don't consider it necessary for this PR, but some improvement for the readability would be nice at some point.

[cmsbuild]

+1
Tested at: daf4a48
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-456e29/6879/summary.html
CMSSW: CMSSW_11_2_X_2020-06-08-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-456e29/6879/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 36
• DQMHistoTests: Total histograms compared: 2780497
• DQMHistoTests: Total failures: 47
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2780400
• DQMHistoTests: Total skipped: 50
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
• Checked 152 log files, 16 edm output root files, 36 DQM output files

[makortel]

+1

[cmsbuild]

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

[silviodonato]

+1