Skip to content
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

Have TkCloner return std::unique_ptr for factory functions #5173

Merged
merged 2 commits into from Sep 6, 2014

Conversation

Dr15Jones
Copy link
Contributor

The static analyzer was complaining about TkCloner returning
non-const pointers from functions. These functions were purely
factory methods so the complaints were not justified. However,
the best way to silence those complaints was to change the return
type to std::unique_ptr since this enforces the ownership transfer
which was merely implicit in the previous API.

This file provides an easy way to hide the details of std::unique_ptr
from ROOT while still being able to use that class as a return type
for member functions parsed by ROOT.
This will be unnecessary when we switch to ROOT 6.
The static analyzer was complaining about TkCloner returning
non-const pointers from functions. These functions were purely
factory methods so the complaints were not justified. However,
the best way to silence those complaints was to change the return
type to std::unique_ptr since this enforces the ownership transfer
which was merely implicit in the previous API.
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2014

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_2_X.

Have TkCloner return std::unique_ptr for factory functions

It involves the following packages:

DataFormats/TrackerRecHit2D
FWCore/Utilities
RecoTracker/TkSeedingLayers
RecoTracker/TransientTrackingRecHit

@nclopezo, @cmsbuild, @Dr15Jones, @StoyanStoynev, @slava77, @ktf can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan, @GiacomoSguazzoni, @rovere, @VinInn, @wddgit, @gpetruc, @cerati, @venturia this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@Dr15Jones
Copy link
Contributor Author

+1

@VinInn
Copy link
Contributor

VinInn commented Sep 5, 2014

+1. thanks Chris. Tracking POG will try to review all static analyzer issues and fix them. (for 73?)

@Dr15Jones
Copy link
Contributor Author

@cmsbuild can the tests be run?

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2014

@StoyanStoynev
Copy link
Contributor

+1
For 764f5ad . Based on jenkins - no diffs and none expected. The code is changed to make use of unique_ptr introducing HideStdUniquePtrFromRoot (see files) concept + few optimizations (release of ownership if nullptr).

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).

ktf added a commit that referenced this pull request Sep 6, 2014
Have TkCloner return std::unique_ptr for factory functions
@ktf ktf merged commit 7fcfdc9 into cms-sw:CMSSW_7_2_X Sep 6, 2014
@Dr15Jones Dr15Jones deleted the useUniquePtrInTkCloner branch September 8, 2014 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants