[Lldb-commits] [PATCH] D146154: [lldb][gnustep] Add minimal GNUstepObjCRuntime plugin for LanguageTypeObjC on non-Apple platforms

[David Chisnall via Phabricator via lldb-commits]

theraven added inline comments.


================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:42
+                                                    LanguageType language) {
+  if (language != eLanguageTypeObjC)
+    return nullptr;
----------------
I'm not familiar with lldb internals, will this exclude 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;
----------------
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.


================
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
----------------
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.


================
Comment at: lldb/test/Shell/Expr/objc-gnustep-print.m:68
+//
+// MEMBERS_OUTSIDE: (lldb) p t->_int
+// MEMBERS_OUTSIDE: (int) $0 = 0
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146154