root
root copied to clipboard
Consider passing TString by value to TNamed ctor
Explain what you would like to see improved and how.
The TNamed ctor currently takes TString by const reference:
TNamed::TNamed(const TString& name, const TString& title)
Reading https://www.cppstories.com/2018/08/init-string-member/ it seems, that it would be a good idea to instead pass it by value and then use move in the ctor?
- TNamed(const TString &name, const TString &title) : fName(name), fTitle(title) { }
+ TNamed(TString name, TString title) : fName(std::move(name)), fTitle(std::move(title)) { }
ROOT version
master
Installation method
none, just looking at the source code.
Operating system
any
Additional context
We're currently looking at improving the ctors of our classes and noticed the double construction of TString, if we don't pass as const char*
.
Nice idea! Can you open a PR for that? This is a very common case indeed, it's nice to optimize it.
By the way, there are clang-tidy
checks to do this automatically.
Nice idea! Can you open a PR for that? This is a very common case indeed, it's nice to optimize it.
Yes, I could. But not before next week (I am quite busy this week).
By the way, there are
clang-tidy
checks to do this automatically.
Oh, I didn't know. Which ones do you mean?
Note that to be effective this change would also require the addition of a move constructor in TString
.
By the way, there are
clang-tidy
checks to do this automatically.Oh, I didn't know. Which ones do you mean?
This looks like one: https://clang.llvm.org/extra/clang-tidy/checks/modernize/pass-by-value.html
Note that to be effective this change would also require the addition of a move constructor in
TString
.
I think this exists already? https://root.cern.ch/doc/master/classTString.html#a97795e61556ec0ced81e15cdfa26a538
TString::TString(TString&& s)
Indeed. I missed it :)