dblaikie added a comment. In D96817#2569852 <https://reviews.llvm.org/D96817#2569852>, @clayborg wrote:
> 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. I think either would be legible/familiar to LLVM developers. I rather like the CRTP, but it's not a make-or-break. I found a couple of instances of cloning APIs across LLVM (incomplete results - grep only gets one so far): clang/include/clang/Sema/TypoCorrection.h: virtual std::unique_ptr<CorrectionCandidateCallback> clone() = 0; lldb/include/lldb/Core/SearchFilter.h: virtual lldb::SearchFilterSP DoCreateCopy() = 0; And neither uses a CRTP for the derive classes, FWIW. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96817/new/ https://reviews.llvm.org/D96817 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
