[Lldb-commits] [PATCH] D76814: Preserve ThreadPlanStacks for unreported threads

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Addressed most of the review comments.  I don't see that using file check would make the plan output checking any easier.  The current function seems clear and easy to use.  Trying to dynamically insert the passed in matches into a filecheck test file and then run file check on that doesn't seem like it would be any easier than what is here.  I don't need any of the substitutions or any of the other fancy things FileCheck provides, that just seems like overkill.  But maybe I'm misunderstanding what you meant.



================
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,
----------------
labath wrote:
> If this is relevant only for os plugins, then it would be good to reflect that in the name as well.
I thought about that.  In the future a regular Process plugin might decide it was too expensive to report all threads as well.  There's nothing in this patch that wouldn't "just work" with that case as well.  Leaving the OS out was meant to indicate this was about how the Process plugin OR any of its helpers (e.g. the OS Plugin) produces threads.


================
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.
----------------
labath wrote:
> this looks like a model example for using `self.filecheck`
I don't see that.  Do you mean write a file check matching file with the contents of the three arrays in this function, and then run file check on that?  Or did you mean convert the call sites into embedded patterns and then call file check on myself?  But then I'd have to also embed the headers in the body such that they came in the right order for all the tests.  Neither of these seem like attractive options to me.  It doesn't seem like it will be hard to maintain this little checker, and I don't see what I would gain by using file check instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76814