JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:305
+ /// named classes.
+ class GetClassInfo {
+ public:
----------------
aprantl wrote:
> GetClassInfo is an odd name for a class. It sounds more like a function. I'm
> also not suggesting ClassInfoFactory :-p
I went with `DynamicClassInfoExtractor`. I'm planning on moving the
corresponding 3 instance variables for the shared cache into a
`SharedCacheClassInfoExtractor` in a follow up patch.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:309
+
+ UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
+ Helper helper);
----------------
aprantl wrote:
> Can this return a nullptr? If not, then this should return a reference.
Yes, if we fail to create the utility function, we'll log the error and return
a nullptr.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:317
+ /// gdb_objc_realized_classes otherwise.
+ static Helper ComputeHelper(AppleObjCRuntimeV2 &runtime);
+
----------------
aprantl wrote:
> Why does this need to be public? I thought the idea of the helper class was
> to hide the actual implementation?
We need to make sure we stick to one way of updating the class info. At some
point I want to move most of `UpdateISAToDescriptorMapDynamic` into this class,
at which this no longer needs to be private. For this patch I tried to keep
unrelated refactorings to a minimum.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h:331
+ /// Utility function to read class info using objc_copyRealizedClassList.
+ std::unique_ptr<UtilityFunction> m_get_class_info2_code;
+ lldb::addr_t m_get_class_info2_args = LLDB_INVALID_ADDRESS;
----------------
aprantl wrote:
> Any better naming ideas?
Not proud of this, but I found that `gdb_objc_realized_classes` and
`objc_copyRealizedClassList` are looked so similar that it was choosing between
the pest and the cholera.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99315/new/
https://reviews.llvm.org/D99315
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
