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
Replaced some deprecated std functions #25521
Conversation
The code-checks are being triggered in jenkins. |
please test |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25521/7718 |
The tests are being triggered in jenkins. |
A new Pull Request was created by @guitargeek (Jonas Rembser) for master. It involves the following packages: DQMOffline/JetMET @perrotta, @cmsbuild, @andrius-k, @kmaeshima, @fwyzard, @schneiml, @Martin-Grunewald, @jfernan2, @slava77, @santocch can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -29,21 +29,12 @@ class AlphaT { | |||
}; | |||
|
|||
|
|||
// ----------------------------------------------------------------------------- |
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.
Why do you remove this constructor ?
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.
Because the constructor does not have to look different anymore for pointer types: std::men_fn
is basically the generalization of std::mem_fun
and std::men_fun_ref
.
From https://en.cppreference.com/w/cpp/utility/functional/mem_fn:
Both references and pointers (including smart pointers) to an object can be used when invoking a std::mem_fn.
I hope I understood this correctly...
Comparison job queued. |
Comparison is ready Comparison Summary:
|
// Wrap function in std::funtion such that member function pointers and other | ||
// functions (lambdas, normal functions, etc.) can be used in the same way | ||
std::function<double(CandidateType const&)> stdFunction{ function }; | ||
for(auto const& x : cands) output.push_back(stdFunction(removeRef(x))); |
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.
does std::decay
simplify this at all?
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.
I'm not familiar with that one and didn't understand immediately how it would help here even after googling. You think it could replace this removeRef
helper, or are you talking about the step with the generalization for member functions and free functions?
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.
Anyway I'll experiment a bit
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.
I didn't have an explicit solution in mind.
Given most recent updates, perhaps https://en.cppreference.com/w/cpp/types/remove_pointer is more inspirational
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.
Ah yes, this also exists! But it does not work with edm::Ptr I imagine.
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.
ok, admittedly, my last comment is going a bit in the backward direction from what is in the PR now and implicitly suggests adding back an equivalent of removeRef(x)
which returns the same for bare T
and Ptr<T>
.
It's more practical to add this adaptor than to reimplement the body of the loop
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
code-checks |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25521/7883
|
+1 |
merge |
Part of my occasional PRs to modernize CMSSW.
I propose to replace the deprecated
std::not1
,std::not2
,std::mem_fun_ref
,std::result_of
in CMSSW with what is recommended in modern C++ (std::mem_fn
,std::not_fn
andstd::invoke_result
).Local matrix tests pass.