[Lldb-commits] [PATCH] D127456: [trace][intelpt] Support system-wide tracing [17] - Some improvements

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp:55-59
+Expected<std::unique_ptr<IntelPTPerThreadProcessTrace>>
 IntelPTPerThreadProcessTrace::Start(const TraceIntelPTStartRequest &request,
                                     ArrayRef<lldb::tid_t> current_tids) {
-  IntelPTProcessTraceUP trace(new IntelPTPerThreadProcessTrace(request));
+  std::unique_ptr<IntelPTPerThreadProcessTrace> trace(
+      new IntelPTPerThreadProcessTrace(request));
----------------
max-kudr wrote:
> wallace wrote:
> > max-kudr wrote:
> > > @wallace, @jj10306
> > > 
> > > We are getting build errors here on CentOS/GNU 7.3.1:
> > > 
> > > ```
> > > /llvm-project/lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp: In static member function 'static llvm::Expected<std::unique_ptr<lldb_private::process_linux::IntelPTPerThreadProcessTrace> > lldb_private::process_linux::IntelPTPerThreadProcessTrace::Start(const lldb_private::TraceIntelPTStartRequest&, llvm::ArrayRef<long unsigned int>)':
> > > /llvm-project/lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp:65:10: error: could not convert 'trace' from 'std::unique_ptr<lldb_private::process_linux::IntelPTPerThreadProcessTrace>' to 'llvm::Expected<std::unique_ptr<lldb_private::process_linux::IntelPTPerThreadProcessTrace> >'
> > >    return trace;
> > >           ^~~~~
> > > ```
> > > 
> > > Can you please fix this?
> > > 
> > Hi Max, I'm aware of this issue because it was reported by someone else. Sadly I wasn't able to install that toolchain on my centos machine (I don't manage it) so I can't reproduce the issue to make sure I can fix it. This issue doesn't happen with other toolchains.
> > 
> > So, I would like to ask for your help to try to fix it. I think the fix might be something as simple as doing
> > 
> > return std::move(trace)
> > or perhaps something like
> > 
> > return Expected<std::unique_ptr<IntelPTPerThreadProcessTrace>>(std::move(trace));
> > At this point I can only rely on you to make sure this is properly fixed
> Hi Walter (@wallace),
> 
> Thank you for following up and proposing fix. I checked and confirm that the code below fixes the issue
> 
> ```
> return std::move(trace)
> ```
> 
> Would you please push this fix to the repository? Thank you!
nice! I will do so now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127456