[SIMULATION] To virtual method inside a final class; make classes non-final #49211

smuzaffar · 2025-10-24T15:03:53Z

LLVM21 has a new warning virtual method inside a final class and can never be overridden. This PR is to fix these warning.
As suggested in #49211 (comment), this PR ake these classes non-final

See #49208

cmsbuild · 2025-10-24T15:04:17Z

cms-bot internal usage

cmsbuild · 2025-10-24T15:06:02Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49211/46541

cmsbuild · 2025-10-24T15:06:21Z

A new Pull Request was created by @smuzaffar for master.

It involves the following packages:

DataFormats/GeometrySurface (simulation)
SimCalorimetry/HcalSimAlgos (simulation)

@civanch, @cmsbuild, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@abdoulline, @bsunanda, @mariadalfonso, @missirol, @mmusich, @rovere, @sameasy this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

smuzaffar · 2025-10-24T15:21:14Z

please test

smuzaffar · 2025-10-24T15:21:47Z

please test for CMSSW_16_0_CLANG_X

cmsbuild · 2025-10-24T15:52:41Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48800/summary.html
COMMIT: 275f0a9
CMSSW: CMSSW_16_0_CLANG_X_2025-10-24-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49211/48800/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation warning when building: See details on the summary page.

civanch · 2025-10-24T17:18:41Z

@smuzaffar , unfortunately, this utility was introduced for dd4hep src/DataFormats/Math/interface/GeantUnits.h

Geant4 peoples commented that it is a potential for problems in future. However, today the fix likely will be needed in many places.

Concerning problem with "final" the alternative solution is to remove "final" declaration from the class - the fix much more simple and will work for sure.

cmsbuild · 2025-10-24T17:29:44Z

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48799/summary.html
COMMIT: 275f0a9
CMSSW: CMSSW_16_0_X_2025-10-24-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49211/48799/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 3 lines from the logs
Reco comparison results: 9 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3939953
DQMHistoTests: Total failures: 31
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3939902
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 218 log files, 188 edm output root files, 51 DQM output files
TriggerResults: found differences in 1 / 49 workflows

civanch · 2025-10-24T19:41:39Z

@smuzaffar , I confused: many compilation warnings but bot approved.

smuzaffar · 2025-10-27T09:27:34Z

@civanch , there are many compilation warning in CLANG_X IBs and this PR only fixes some of those that is why you still see many warnings. For Default IB e.g. https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48799/build-logs/ there are no warnings.

Concerning problem with "final" the alternative solution is to remove "final" declaration from the class - the fix much more simple and will work for sure.

OK, I will update the PR to remove the final keyword from class instead of removing virtual from methods

…-final

cmsbuild · 2025-10-27T12:17:04Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48829/summary.html
COMMIT: 3785fac
CMSSW: CMSSW_16_0_CLANG_X_2025-10-26-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49211/48829/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation warning when building: See details on the summary page.

smuzaffar · 2025-10-27T12:18:51Z

please test

smuzaffar · 2025-10-27T12:25:51Z

please test with #49212 for CMSSW_16_0_CLANG_X

cmsbuild · 2025-10-27T14:31:04Z

+1

Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48831/summary.html
COMMIT: 3785fac
CMSSW: CMSSW_16_0_X_2025-10-27-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49211/48831/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 1 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3939953
DQMHistoTests: Total failures: 66
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3939867
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 218 log files, 188 edm output root files, 51 DQM output files
TriggerResults: no differences found

civanch · 2025-10-27T16:50:29Z

+1

cmsbuild · 2025-10-27T16:50:57Z

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. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: #49212

cmsbuild · 2025-10-27T19:12:42Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1c27d8/48832/summary.html
COMMIT: 3785fac
CMSSW: CMSSW_16_0_CLANG_X_2025-10-26-2300/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49211/48832/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 199 lines to the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 92012 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3939953
DQMHistoTests: Total failures: 456318
DQMHistoTests: Total nulls: 324
DQMHistoTests: Total successes: 3483291
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -1.8600000000000003 KiB( 50 files compared)
DQMHistoSizes: changed ( 10224.0 ): -0.075 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 13034.0 ): -2.970 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 17034.0 ): 4.203 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 2024.0010001 ): 0.020 KiB JetMET/SUSYDQM
DQMHistoSizes: changed ( 250202.181 ): 0.650 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 7.3 ): -3.688 KiB SiStrip/MechanicalView
Checked 218 log files, 188 edm output root files, 51 DQM output files
TriggerResults: found differences in 16 / 49 workflows

smuzaffar · 2025-10-28T08:47:34Z

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. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: #49212

#49212 is independent PR , so this PR can go in with out #49212

ftenchini · 2025-10-28T10:44:02Z

+1

cmsbuild added this to the CMSSW_16_0_X milestone Oct 24, 2025

cmsbuild added simulation-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 24, 2025

smuzaffar mentioned this pull request Oct 24, 2025

llvm21: virtual method inside a final class and can never be overridden #49208

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 24, 2025

cmsbuild added tests-started and removed tests-pending labels Oct 24, 2025

cmsbuild added tests-approved and removed tests-started labels Oct 24, 2025

[SIMULATION] To virtual method inside a final class; make classes non…

3785fac

…-final

smuzaffar force-pushed the simulation-llvm21-virtual-method-final-class branch from 275f0a9 to 3785fac Compare October 27, 2025 11:02

smuzaffar changed the title ~~[SIMULATION] Fix for virtual method inside a final class~~ [SIMULATION] To virtual method inside a final class; make classes non-final Oct 27, 2025

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Oct 27, 2025

cmsbuild added the tests-started label Oct 27, 2025

cmsbuild added tests-rejected and removed tests-started labels Oct 27, 2025

cmsbuild added tests-started and removed requires-external tests-rejected labels Oct 27, 2025

cmsbuild added the requires-external label Oct 27, 2025

cmsbuild added tests-approved and removed tests-started labels Oct 27, 2025

cmsbuild added simulation-approved fully-signed and removed simulation-pending pending-signatures labels Oct 27, 2025

cmsbuild mentioned this pull request Oct 27, 2025

[RECONSTRUCTION] Fix for virtual method inside a final class #49212

Merged

cmsbuild added orp-approved and removed orp-pending labels Oct 28, 2025

cmsbuild merged commit 709eee2 into cms-sw:master Oct 28, 2025
16 checks passed

[SIMULATION] To virtual method inside a final class; make classes non-final #49211

[SIMULATION] To virtual method inside a final class; make classes non-final #49211

Uh oh!

Conversation

smuzaffar commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Oct 24, 2025

Uh oh!

cmsbuild commented Oct 24, 2025

Uh oh!

smuzaffar commented Oct 24, 2025

Uh oh!

smuzaffar commented Oct 24, 2025

Uh oh!

cmsbuild commented Oct 24, 2025

Build

Uh oh!

civanch commented Oct 24, 2025

Uh oh!

cmsbuild commented Oct 24, 2025

Comparison Summary

Uh oh!

civanch commented Oct 24, 2025

Uh oh!

smuzaffar commented Oct 27, 2025

Uh oh!

cmsbuild commented Oct 27, 2025

Build

Uh oh!

smuzaffar commented Oct 27, 2025

Uh oh!

smuzaffar commented Oct 27, 2025

Uh oh!

cmsbuild commented Oct 27, 2025

Comparison Summary

Uh oh!

civanch commented Oct 27, 2025

Uh oh!

cmsbuild commented Oct 27, 2025

Uh oh!

cmsbuild commented Oct 27, 2025

Comparison Summary

Uh oh!

smuzaffar commented Oct 28, 2025

Uh oh!

ftenchini commented Oct 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smuzaffar commented Oct 24, 2025 •

edited

Loading

cmsbuild commented Oct 24, 2025 •

edited

Loading