[Lldb-commits] [PATCH] D127252: [lldb] Use objc_getRealizedClassList_trylock if available

[Jim Ingham via Phabricator via lldb-commits]

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

One grammatical correction and a couple of questions.  But I'm also fine with the way it is.



================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2017
 
+  // Deallocate the memory we allocated for the Class array
+  if (class_buffer_addr != LLDB_INVALID_ADDRESS)
----------------
Is it worth doing these as exit_scope dingii so we don't have to worry about returning early w/o doing them?  There's a fairly long way from the AllocateMemory to the DeallocateMemory.


================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:318
+  /// objc_getRealizedClassList_trylock. The RealizedClassList variants are
+  /// preferred because it includes lazily named classes, but they are not
+  /// always available or safe to call.
----------------
grammar


================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:366
 
+    /// Helper to read class info using objc_getRealizedClassList_trylock.
+    struct objc_getRealizedClassList_trylock_helper {
----------------
You didn't do this, so it's not necessary to address it, but why do we have three identical structure definitions with just different names?  Were they going to be different at some point then ended up not being?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127252