[Lldb-commits] [PATCH] D39733: Simplify NativeProcessProtocol::GetArchitecture/GetByteOrder

[Eugene Zemtsov via Phabricator via lldb-commits]

eugene added inline comments.


================
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:104
 
-  virtual bool GetArchitecture(ArchSpec &arch) const = 0;
+  virtual const ArchSpec &GetArchitecture() const = 0;
 
----------------
labath wrote:
> eugene wrote:
> > Why return reference instead of a value?
> I'd actually reverse that: Why value instead of a reference ? :D
> 
> Returning by reference allows us to potentially avoid a copy. It does mean that the process object has to hold the archspec as a member, but that is currently not an issue, and we can always change that.
>  Why value instead of a reference ? :D
IMO returning by value is a default option, for 2  reasons:
1. References and pointers introduce implicit lifetime dependencies (prone to use-after-free)
2. Value can change over time, and client might not expect it to happen.

Potentially avoid a copy in place where a copy was made previously and didn't seem to hurt anybody seems like a bad trade-off for one more place where we can get a dangling pointer. 



https://reviews.llvm.org/D39733