labath added a comment.
I still don't know much about thread plans, but I didn't see anything that
would stand out. All my comments are fairly superficial.
================
Comment at: lldb/source/Target/TargetProperties.td:183
Desc<"A path to a python OS plug-in module file that contains a
OperatingSystemPlugIn class.">;
+ def PluginReportsAllThreads: Property<"plugin-reports-all-threads",
"Boolean">,
+ Global,
----------------
If this is relevant only for os plugins, then it would be good to reflect that
in the name as well.
================
Comment at:
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp:1-7
+//===-- main.cpp ------------------------------------------------*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
----------------
We don't put licence headers on tests.
================
Comment at:
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp:20
+#include <thread>
+#include <unistd.h>
+
----------------
unistd.h is not portable, and it doesn't look like you are using anything from
it.
================
Comment at:
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp:33
+ g_cv.notify_one();
+ g_cv.wait(func_lock);
+}
----------------
The synchronization here seems to rely on the fact that there will be no
spurious wakeups. The simplest way to guard against that is to pass a lambda
into the wait function `g_cv.wait(func_lock, [&] { return g_value == 2; })`
================
Comment at:
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/main.cpp:38-41
+ step_out_of_here(); // Expect to stop here after step-out (clang)
+
+ // Return
+ return NULL; // Expect to stop here after step-out (icc and gcc)
----------------
The clang/gcc/icc anotations are not used, and who knows if they are even
accurate at this point.
================
Comment at:
lldb/test/API/functionalities/thread_plan/TestThreadPlanCommands.py:23
+
+ def check_list_output(self, command, active_plans = [], completed_plans =
[], discarded_plans = []):
+ # Check the "thread plan list" output against a list of active &
completed and discarded plans.
----------------
this looks like a model example for using `self.filecheck`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76814/new/
https://reviews.llvm.org/D76814
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
