[Lldb-commits] [PATCH] D123421: Handle the new "selector-specific stub" ObjC Method dispatch method

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I also made one functional change. Some code was checking `!m_lookup_implementation_function_code.empty()` to see if we had found an implementation function.  That won't work if I put the common prefix into this ivar in the constructor, so I change that to leave the member empty till we actually figure out that we want one or the other of the prefixes.  This is an error path that you have to intentionally mess things up to get to, but still it's worth preserving this signal.



================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:637-643
     // Also we will use the version of the lookup code that doesn't rely on the
     // stret version of the function.
-    m_lookup_implementation_function_code =
-        g_lookup_implementation_no_stret_function_code;
+    m_lookup_implementation_function_code.append(
+        g_lookup_implementation_no_stret_function_code);
   } else {
-    m_lookup_implementation_function_code =
-        g_lookup_implementation_with_stret_function_code;
+    m_lookup_implementation_function_code.append(
+        g_lookup_implementation_with_stret_function_code);
----------------
JDevlieghere wrote:
> I don't understand why we need to have two versions of the code. IIUC, if there's a "stret return lookup function", then if we pass `is_stret == true` we'll use it. Otherwise we'll unconditionally use the straight lookup. Why do we need two variants of the code at all? Can't we pass false to `is_stret` and achieve the same result with the `g_lookup_implementation_with_stret_function_code` variant? 
On the systems that don't use stret returns, class_getMethodImplementation_stret doesn't exist.  So we would either have to have two versions of that part of the code, or passing function pointers to the two functions (making them the same in the no _stret case) or do some dlsym type ugliness.

This solution is straightforward, and reducing complexity in these Utility functions is a virtue.  Having two copies of all the code was a bit bogus, but this solution is straightforward and easy to debug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123421