[Lldb-commits] [PATCH] D122429: [lldb/Plugin] Sort the ScriptedProcess' thread list before creating threads

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGMT



================
Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:321-342
+  auto sort_keys = [&sorted_threads,
+                    &thread_info_sp](StructuredData::Object *item) -> bool {
+    if (!item)
+      return false;
+
+    llvm::StringRef key = item->GetStringValue();
+    size_t idx = 0;
----------------
mib wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > Instead of this callback, can we simplify the code by iterating over the keys and populate the map that way?
> > We don't have iterators to iterate over `StructuredData` containers with a range-based loop, so we can only iterate over them using indices ...
> > 
> > I find it ungracious to do it that way because we have to call `GetItemAtIndex(idx)` to get the `StructuredData::Object` element from the array, that's why I used the `ForEach` method instead.
> > 
> > I think we can improve StructuredData containers to be more customizable and operate better with the `std`/`adl` utilities, it should be done separately.
> I think we've discussed this offline, but if we made the `StructuredData::Array` templated (with a default template parameter), we could prevent having to sort ourself the thread dictionary, and loop twice over it (which annoys me quite a bit 😅😅)
Okay, that's unfortunate. Thanks for the explanation!


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

https://reviews.llvm.org/D122429