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
Removed unneeded uses of unique_ptr from GsfElectronAlgo #25977
Conversation
In addition to being a code cleanup, this avoids an apparent compiler bug in clang 7 where default constructors are required but missing.
This fixes the linking problem seen in the CLANG IBs |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25977/8482
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages: RecoEgamma/EgammaElectronAlgos @cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
Thank you, I missed that in my last PR. |
Are the remaining unique pointers also causing these clang errors by the way? They are a bit more involving to get rid of. |
Only 1 caused a clang problem. The ones I didn't change seemed to be a reasonable use of |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@Dr15Jones , for my own education:
|
GeneralData and EventSetupData were being 'new'ed in the constructor and 'delete'd in the destructor thereby having a lifetime equal to the parent class. That is exactly the same lifetime as member data. EventData and ElectronData are 'new'ed and 'delete'd each event. |
Right.
Thank you Chris!
Chris Jones <notifications@github.com> ha scritto:
… @perrotta
> (Saying it differently) why didn't you remove it also for EventData
> and ElectronData?
GeneralData and EventSetupData were being 'new'ed in the constructor
and 'delete'd in the destructor thereby having a lifetime equal to
the parent class. That is exactly the same lifetime as member data.
EventData and ElectronData are 'new'ed and 'delete'd each event.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#25977 (comment)
|
ping |
This change is needed to fix the broken builds on CLANG |
+1
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
In addition to being a code cleanup, this avoids an apparent compiler bug in clang 7 where default constructors are required but missing.