[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 13 08:24:13 PDT 2019

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.

Comment at: source/Core/ValueObject.cpp:1702-1724
+      return runtime->IsRuntimeSupportValue(*this) &&
+             !runtime->IsWhitelistedRuntimeValue(GetName());
+    // It's possible we have more than one language involved here. For example,
+    // in ObjC `_cmd` is a whitelisted runtime variable name, but
+    // GetObjectRuntimeLanguage will say it's a C variable since it's just a
+    // cstring.
Seems like this should still just make one call. The two calls to IsRuntimeSupportValue followed by IsWhitelistedRuntimeValue seems like a bunch of hard to read code. We should make just one call to each runtime and do all the work needed from within there. So I vote to revert this change and do the work in each IsRuntimeSupportValue method. See additional comments in CPPLanguageRuntime::IsRuntimeSupportValue() that was removed. 

Comment at: source/Target/CPPLanguageRuntime.cpp:59
-    return false;
-  return !ObjCLanguageRuntime::IsWhitelistedRuntimeValue(name);
Seems like we should restore this function and just get the ObjC language runtime from the process, and if it exists, call IsWhitelistedRuntimeValue on it?

auto objc_runtime = ObjCLanguageRuntime::Get(m_process);
if (objc_runtime)
  return objc_runtime->IsWhitelistedRuntimeValue(name);
return false;



More information about the lldb-commits mailing list