On 05/03/2019 17:19, Jonas Devlieghere wrote:
Sounds good, I’ll make this change before landing, unless you have
further comments?
On Tue, Mar 5, 2019 at 4:29 AM Pavel Labath via Phabricator
<revi...@reviews.llvm.org <mailto:revi...@reviews.llvm.org>> wrote:
labath added a comment.
The copy constructors and assignment operators are very repetitive.
What would you say to a function like
template<typename T>
void clone(std::unique_ptr<T> &dest, const std::unique_ptr<T> &src) {
if (&dest == &src)
return;
if (src)
dest = llvm::make_unique<T>(*src);
else
dest.reset();
}
(and a similar one for shared_ptrs)? Then both copy constructors and
assignment operators could be just implemented as `clone(m_whatever,
rhs.m_whatever);`. (Assuming we don't care about pessimizing the
self-assignment case this could even be `m_whatever =
clone(rhs.m_whatever);`)
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58946/new/
https://reviews.llvm.org/D58946
Yeah, that's fine. The only thing I could possibly add is that this
approach depends on not "mirroring" the default constructor, which is
the thing you were trying to the in the original patch, and I don't
think it's right or necessary.
It's not right because I think a copy constructor should make a exact
copy, regardless of what the default constructor of the same object
would create.
And it's not necessary because if the default constructor already
initializes the unique_ptr member (which is the thing you are trying to
mirror), then you should not ever encounter a object with an empty
unique_ptr to copy from.
pl
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
