sgraenitz marked 3 inline comments as done.
sgraenitz added a comment.
Hi David, thanks for your notes. Please find responses inline.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:42
+ LanguageType language) {
+ if (language != eLanguageTypeObjC)
+ return nullptr;
----------------
theraven wrote:
> I'm not familiar with lldb internals, will this exclude ObjC++?
Good question. There is also `eLanguageTypeObjC_plus_plus`, but the plugin
scanner e.g. seems to use `eLanguageTypeObjC` for both. I'd like to keep it
like that until I actually start testing ObjC++.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:47
+ const llvm::Triple &TT = process->GetTarget().GetArchitecture().GetTriple();
+ if (TT.getVendor() == llvm::Triple::VendorType::Apple)
+ return nullptr;
----------------
theraven wrote:
> I never finished the v2 ABI support for Mach-O, but v1 works on Apple
> platforms and v2 will eventually. We definitely shouldn't be the default
> here, but it would be nice not to prevent it working on macOS.
For the moment it would be nice to stay away from potential conflicts, since
they won't make reviews easier. We can still add this later on if there is
demand.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:131
+ ExecutionContext &exe_ctx) {
+ // TODO: This function is supposed to check whether an ObjC selector is
+ // present for an object. Might be implemented similar as in the Apple V2
----------------
theraven wrote:
> I'm not sure how this is meant to work. I'd expect this to call one of the
> runtime's introspection functions. I believe the Apple version has variants
> of these that run without acquiring locks, which can be used in the debugger.
> We can add these quite easily if necessary.
The Apple V2 runtime has two ways to do it:
https://github.com/llvm/llvm-project/blob/release/16.x/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp#L1179
I experimented with the one that doesn't require `void
*gdb_class_getClass(void*)` from the runtime library, but the JIT kept on
crashing. So, for this first version I opted for a minimal implementation that
doesn't do any selector checks. Let's look at it in another review.
================
Comment at: lldb/test/Shell/Expr/objc-gnustep-print.m:68
+//
+// MEMBERS_OUTSIDE: (lldb) p t->_int
+// MEMBERS_OUTSIDE: (int) $0 = 0
----------------
theraven wrote:
> It's not clear from this test if it's correctly computing the offsets. I
> suspect that it isn't, since I don't see any reference to the ivar offset
> variables.
What exactly would you expect to see in the output?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146154/new/
https://reviews.llvm.org/D146154
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
