mib added a comment. In D134033#3933936 <https://reviews.llvm.org/D134033#3933936>, @labath wrote:
> I kinda like it. One thing that I think would help with the readability is if > the "transformation" methods were grouped according to the type of the object > being transformed, rather than according to the direction. So something like: > > // general transform > // general reverse > // Status transform > // Status reverse > > instead of > > // general transform > // Status transform > // general reverse > // Status reverse > > Also I don't think that these structs (`transformation`, > `reverse_transformation`) wrapping the transformation functions are really > necessary. It's true that one cannot partially specialize functions, but one > of the reasons for that is this is normally not necessary -- regular function > overloading <https://godbolt.org/z/114KeeK3f> can do most of that as well. Thanks for the feedback! I really appreciate it :) For now I reordered the structs as you suggested but I'm thinking of moving all the transformation related code to an anonymous namespace above the class. I'll do that in a follow-up patch :) ================ Comment at: lldb/include/lldb/API/SBError.h:95 private: - std::unique_ptr<lldb_private::Status> m_opaque_up; + std::shared_ptr<lldb_private::Status> m_opaque_sp; ---------------- labath wrote: > This is technically an ABI break (changes `sizeof(SBError)`). I don't care, > but someone might. Luckily, I didn't have to change this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134033/new/ https://reviews.llvm.org/D134033 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
