-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Apply rule-of-5 to ESPreFunctorDecorator #44483
Apply rule-of-5 to ESPreFunctorDecorator #44483
Conversation
This avoids a compiler warning under clang.
please test |
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44483/39572
|
A new Pull Request was created by @Dr15Jones for master. It involves the following packages:
@smuzaffar, @makortel, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
ESPreFunctorDecorator(ESPreFunctorDecorator&&) = default; | ||
ESPreFunctorDecorator(ESPreFunctorDecorator const&) = default; | ||
ESPreFunctorDecorator& operator=(const ESPreFunctorDecorator&) = delete; // stop default | ||
ESPreFunctorDecorator& operator=(ESPreFunctorDecorator&&) = delete; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double check, this class is copyable and movable via constructor, but not via assignment? I.e. assigning to a caller_
(when not initializing it) is what we want to avoid? But copying caller_
as such is fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original code stopped the regular op= but the compiler warning complained about the default copy constructor. To make the code work, I had to require the copy constructor (and figured move constructor would be fine). Then I just picked the move op= to be treated like the regular op=.
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32a07f/38283/summary.html Comparison SummarySummary:
|
+core |
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. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This avoids a compiler warning under clang.
PR validation:
Compiling code under CMSSW_14_1_CLANG_X_2024-03-19-2300 no longer issues a warning.