[Lldb-commits] [PATCH] D96817: Fix deep copying for OptionValue classes

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D96817#2569595 <https://reviews.llvm.org/D96817#2569595>, @dblaikie wrote:

>> CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.
>
> I would've thought it'd be pretty easy to accidentally miss either of these - I think the CRTP helper ensures consistency of implementation (harder to accidentally slice/copy the wrong type/etc. But I'm not a code owner/major contributor to lldb specifically, so probably more up to other developers who are.

Whatever is the safest and most llvm like is probably the best approach IMHO. So maybe stick with a solution that will be familiar to LLVM developers if this current approach isn't. I am open to other suggestions if others feel strongly otherwise.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96817/new/

https://reviews.llvm.org/D96817