[Lldb-commits] [PATCH] D67965: Have ABI plugins vend llvm MCRegisterInfo data

[Pavel Labath via Phabricator via lldb-commits]

labath marked 3 inline comments as done.
labath added inline comments.


================
Comment at: include/lldb/Target/ABI.h:141
 protected:
-  // Classes that inherit from ABI can see and modify these
-  ABI(lldb::ProcessSP process_sp) {
-    if (process_sp.get())
-        m_process_wp = process_sp;
+  ABI(lldb::ProcessSP process_sp, std::unique_ptr<llvm::MCRegisterInfo> info_up)
+      : m_process_wp(process_sp), m_mc_register_info_up(std::move(info_up)) {
----------------
tatyana-krasnukha wrote:
> Since this and derived class's constructors are protected/private it can make sense to make their parameters r-value and not rely on copy elision.
Technically, this is not copy elision, since std::move supresses that. :)
But anyway, I generally try to avoid overusing rvalue references as they tend to make one wonder whether there is something funny going on (and in particular with std::unique_ptr, a rvalue reference can have subtly different behavior than a plain object). I also believe the advice given (by some experts at least) since c++11 is to prefer plain objects where that makes sense.


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

https://reviews.llvm.org/D67965