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

[Max Kudryavtsev via Phabricator via lldb-commits]

max-kudr 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));
----------------
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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127456