[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) &&
+ // 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);
CHANGES SINCE LAST ACTION
More information about the lldb-commits