Enable misc-definitions-in-headers clang-tidy check

[makortel]

PR description:

This PR suggests to enable misc-definitions-in-headers clang-tidy check to find and fix non-inline function definitions in headers (based on a recent encounter of one slipping through #33319 (comment)).

PR validation:

Tested that a header defining a function without inline gets flagged and fixed. The impact on the entire CMSSW (excluding test, macros, bin directories) is shown in #33366.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33365/21954

• This PR adds an extra 12KB to repository

[cmsbuild]

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

It involves the following packages:

.clang-tidy

@makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[makortel]

To include the .icc as header file extensions I tried several variations of

CheckOptions:
...
  - key:             misc-definitions-in-headers.HeaderFileExtensions
    value:           'h,hh,hpp,hxx,icc'

with comma and semicolon separators and with and without an "empty element".

[smuzaffar]

@makortel , build rules now properly process the changes suggested by this check. How should we proceed? I can make PRs ( one per category) to apply this check on full cmssw. Only 66 header files are changed due to this check but over all there are around 250 files which still needs clang tidy/format checks.

https://cmssdt.cern.ch/jenkins/view/Jenkins%20Tests/job/jenkins-test-code-format/71/console job ran clang-tidy/format of full cmssw.

[makortel]

I think the usual "PR per category" would be good way to go.

Should we try to include the .icc files now or leave that for later?

[smuzaffar]

yes we should add icc extension via misc-definitions-in-headers.HeaderFileExtensions now.

I will open PRs next week with tidy/formt changes for full cmssw

[makortel]

Should I add

  - key:             misc-definitions-in-headers.HeaderFileExtensions
    value:           'h,hh,hpp,hxx,icc'

to CheckOptions: already now then?

[smuzaffar]

yes please

[makortel]

Done. (but note that I did not test now this exact version of the file)

[smuzaffar]

No, worries @makortel . I am going to use this PR to apply clang-tidy changes .

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33365/23120

• This PR adds an extra 12KB to repository

[cmsbuild]

Pull request #33365 was updated. @makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please check and sign again.

[smuzaffar]

@makortel , looks like my last set of PRs has missed to include this change. That is whjy there are no fixes proposed by the check for those PRs. Once the currect set of PRs merged then I will do another round ( should be a small change set as by then the rest of cmssw should be all clean) with this change

[makortel]

Sounds good, thanks!

[smuzaffar]

please test
@makortel , all my PRs to apply this check are now merged. I think it is time to merge this too

[makortel]

Sounds good, thanks!

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b5b01e/16212/summary.html
COMMIT: e4a1c20
CMSSW: CMSSW_12_0_X_2021-06-23-1600/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33365/16212/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

• No significant changes to the logs found
• Reco comparison results: 1260 differences found in the comparisons
• DQMHistoTests: Total files compared: 38
• DQMHistoTests: Total histograms compared: 2785631
• DQMHistoTests: Total failures: 3669
• DQMHistoTests: Total nulls: 19
• DQMHistoTests: Total successes: 2781921
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -45.703 KiB( 37 files compared)
• DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
• DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
• Checked 160 log files, 37 edm output root files, 38 DQM output files
• TriggerResults: no differences found

[smuzaffar]

+core

[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, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[qliphy]

+1