[Lldb-commits] [PATCH] D126402: Fix conversion error to Expected

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre created this revision.
thopre added reviewers: wallace, jj10306.
Herald added a project: All.
thopre requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

On Ubuntu 18.04 with GCC 7.5 Intel trace code fails to build due to
failure to convert from
lldb_private::process_linux::IntelPTPerThreadProcessTraceUP to
Expected.
This commit explicitely marks those unique_ptr values as being moved
which fixes the conversion error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126402

Files:
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -318,7 +318,7 @@
 }
 IntelPTSingleBufferTraceUP trace_up(
 new IntelPTSingleBufferTrace(std::move(*perf_event), initial_state));
-return trace_up;
+return std::move(trace_up);
   } else {
 return perf_event.takeError();
   }
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -78,7 +78,7 @@
 error = joinErrors(std::move(error), trace->TraceStart(tid));
   if (error)
 return std::move(error);
-  return trace;
+  return std::move(trace);
 }
 
 Error IntelPTCollector::TraceStart(const TraceIntelPTStartRequest &request) {


Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -318,7 +318,7 @@
 }
 IntelPTSingleBufferTraceUP trace_up(
 new IntelPTSingleBufferTrace(std::move(*perf_event), initial_state));
-return trace_up;
+return std::move(trace_up);
   } else {
 return perf_event.takeError();
   }
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -78,7 +78,7 @@
 error = joinErrors(std::move(error), trace->TraceStart(tid));
   if (error)
 return std::move(error);
-  return trace;
+  return std::move(trace);
 }
 
 Error IntelPTCollector::TraceStart(const TraceIntelPTStartRequest &request) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126402: Fix conversion error to Expected

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre added a comment.

In D126402#3538078 , @wallace wrote:

> thank you!
>
> I'm surprised the compiler didn't do this automatically

As mentioned, this is GCC 7.5. It might be different with a newer version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126402

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126402: Fix conversion error to Expected

[Thomas Preud'homme via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e757c6b500d: Fix conversion error to Expected (authored by 
thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126402

Files:
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -318,7 +318,7 @@
 }
 IntelPTSingleBufferTraceUP trace_up(
 new IntelPTSingleBufferTrace(std::move(*perf_event), initial_state));
-return trace_up;
+return std::move(trace_up);
   } else {
 return perf_event.takeError();
   }
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -78,7 +78,7 @@
 error = joinErrors(std::move(error), trace->TraceStart(tid));
   if (error)
 return std::move(error);
-  return trace;
+  return std::move(trace);
 }
 
 Error IntelPTCollector::TraceStart(const TraceIntelPTStartRequest &request) {


Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -318,7 +318,7 @@
 }
 IntelPTSingleBufferTraceUP trace_up(
 new IntelPTSingleBufferTrace(std::move(*perf_event), initial_state));
-return trace_up;
+return std::move(trace_up);
   } else {
 return perf_event.takeError();
   }
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -78,7 +78,7 @@
 error = joinErrors(std::move(error), trace->TraceStart(tid));
   if (error)
 return std::move(error);
-  return trace;
+  return std::move(trace);
 }
 
 Error IntelPTCollector::TraceStart(const TraceIntelPTStartRequest &request) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre added a comment.

Is there no way to split this patch further? It's going to be hard finding 
someone who can review something so big. If there's no way to split it in 
incremental changes, you could perhaps split per subsystem only for review and 
refer to this diff for CI as well as when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109345

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D102630: [lit] Stop using PATH to lookup clang/lld/lldb unless requested

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre added a comment.

In D102630#2764705 , @dblaikie wrote:

> @thopre - as an aside: It'd be helpful if you could include some text in the 
> text box when marking something "approved" through phabricator. There's a 
> bug/limitation that approvals without text don't produce email to the mailing 
> list - so it looks like a patch hasn't been reviewed yet (or has been 
> committed without review).

Oh sorry, I was not aware of that limitation. Will be careful to include a 
message in the future


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102630

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre added a comment.

In D98179#2621706 , @davezarzycki 
wrote:

> I've made all of the requested changes to date. Two notes:
>
> 1. One cannot simply omit the first argument to `split()` if the second is 
> provided. As I just learned, python will complain. I switched the first 
> parameter to `None` as a compromise.
> 2. For the record, I disagree with the use of the word "run" over "start" but 
> this isn't something that I care enough about to resist changing the patch. 
> Strictly speaking, `lit` only controls when tests start, not anything about 
> how they run. With enough cores, tests will complete in a fairly predictable 
> order despite any effort to start tests in a different order (or randomly).

That's odd, the documentation 
 even contains 
such an example:

  For example:
  >>>
  
  >>> '1,2,3'.split(',')
  ['1', '2', '3']
  >>> '1,2,3'.split(',', maxsplit=1)
  ['1', '2,3']
  >>> '1,2,,3,'.split(',')
  ['1', '2', '', '3', '']


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68606: [test] Split LLDB tests into filecheck, unittest and dotest.

[Thomas Preud'homme via Phabricator via lldb-commits]

thopre added a comment.

Shouldn't the CMake target remain check-lldb-unit to be consistant with 
check-llvm-unit/check-clang-unit etc.?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68606



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits