[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux

[Ravitheja Addepally via Phabricator via lldb-commits]

ravitheja added inline comments.


================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:277
+
+  llvm::DenseMap<lldb::tid_t, ProcessorTraceMonitorSP>
+      m_processor_trace_monitor;
----------------
labath wrote:
> ravitheja wrote:
> > labath wrote:
> > > I'd like to downgrade these to unique pointers to ProcessTraceMonitor. There's no reason for these to ever outlive or escape the process instance, so it's natural to say they are strongly owned by it. In other places where you use ProcessorTraceMonitorSP you can just use regular pointers or references (depending on whether they can be null or not).
> > Hi, I don't see the advantage of changing to unique pointers ? coz when the process dies they will be destroyed anyhow, plus using shared pointers makes it easier for functions operating with the ProcessTraceMonitor to work.
> It makes it clear that the Process is the owner of these objects (and not for example "sharing" them with anyone else). Plus you should use the simplest tool that gets the job done and unique_ptr is definitely simpler. So I'd reverse the question: If there is no need for using shared_ptr, why do it?
> 
> I disagree with the statement that it makes it harder for the functions to work. Please provide an example.
Ok I was thinking that working with shared_pointers was a bit cleaner approach than working with references or raw pointers, but in this case won't make much of a difference.

I will make them unique pointers. No problem.


https://reviews.llvm.org/D33674