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

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

Reply via email to