-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LLDB] Reapply refactored CPlusPlusLanguage::MethodName to break lldb-server dependencies #135033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Dmitry Vasilyev (slydiman) ChangesThe original PR is #132274. Patch is 38.35 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135033.diff 16 Files Affected:
diff --git a/lldb/include/lldb/Core/Mangled.h b/lldb/include/lldb/Core/Mangled.h
index 5988d919a89b8..7db63eeeb6ee0 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -246,6 +246,8 @@ class Mangled {
/// for s, otherwise the enumerator for the mangling scheme detected.
static Mangled::ManglingScheme GetManglingScheme(llvm::StringRef const name);
+ static bool IsMangledName(llvm::StringRef name);
+
/// Decode a serialized version of this object from data.
///
/// \param data
diff --git a/lldb/include/lldb/Core/RichManglingContext.h b/lldb/include/lldb/Core/RichManglingContext.h
index 3b79924e88a9a..50ec2ae361098 100644
--- a/lldb/include/lldb/Core/RichManglingContext.h
+++ b/lldb/include/lldb/Core/RichManglingContext.h
@@ -12,6 +12,7 @@
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private.h"
+#include "lldb/Target/Language.h"
#include "lldb/Utility/ConstString.h"
#include "llvm/ADT/Any.h"
@@ -67,11 +68,7 @@ class RichManglingContext {
char *m_ipd_buf;
size_t m_ipd_buf_size = 2048;
- /// Members for PluginCxxLanguage
- /// Cannot forward declare inner class CPlusPlusLanguage::MethodName. The
- /// respective header is in Plugins and including it from here causes cyclic
- /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
- llvm::Any m_cxx_method_parser;
+ std::unique_ptr<Language::MethodName> m_cxx_method_parser;
/// Clean up memory when using PluginCxxLanguage
void ResetCxxMethodParser();
@@ -81,15 +78,6 @@ class RichManglingContext {
/// Uniform handling of string buffers for ItaniumPartialDemangler.
llvm::StringRef processIPDStrResult(char *ipd_res, size_t res_len);
-
- /// Cast the given parser to the given type. Ideally we would have a type
- /// trait to deduce \a ParserT from a given InfoProvider, but unfortunately we
- /// can't access CPlusPlusLanguage::MethodName from within the header.
- template <class ParserT> static ParserT *get(llvm::Any parser) {
- assert(parser.has_value());
- assert(llvm::any_cast<ParserT *>(&parser));
- return *llvm::any_cast<ParserT *>(&parser);
- }
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h
index b699a90aff8e4..d46969cb3b4e4 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -214,6 +214,104 @@ class Language : public PluginInterface {
return std::vector<Language::MethodNameVariant>();
};
+ class MethodName {
+ public:
+ MethodName() {}
+
+ MethodName(ConstString full)
+ : m_full(full), m_basename(), m_context(), m_arguments(),
+ m_qualifiers(), m_return_type(), m_scope_qualified(), m_parsed(false),
+ m_parse_error(false) {}
+
+ virtual ~MethodName() {};
+
+ void Clear() {
+ m_full.Clear();
+ m_basename = llvm::StringRef();
+ m_context = llvm::StringRef();
+ m_arguments = llvm::StringRef();
+ m_qualifiers = llvm::StringRef();
+ m_return_type = llvm::StringRef();
+ m_scope_qualified.clear();
+ m_parsed = false;
+ m_parse_error = false;
+ }
+
+ bool IsValid() {
+ if (!m_parsed)
+ Parse();
+ if (m_parse_error)
+ return false;
+ return (bool)m_full;
+ }
+
+ ConstString GetFullName() const { return m_full; }
+
+ llvm::StringRef GetBasename() {
+ if (!m_parsed)
+ Parse();
+ return m_basename;
+ }
+
+ llvm::StringRef GetContext() {
+ if (!m_parsed)
+ Parse();
+ return m_context;
+ }
+
+ llvm::StringRef GetArguments() {
+ if (!m_parsed)
+ Parse();
+ return m_arguments;
+ }
+
+ llvm::StringRef GetQualifiers() {
+ if (!m_parsed)
+ Parse();
+ return m_qualifiers;
+ }
+
+ llvm::StringRef GetReturnType() {
+ if (!m_parsed)
+ Parse();
+ return m_return_type;
+ }
+
+ std::string GetScopeQualifiedName() {
+ if (!m_parsed)
+ Parse();
+ return m_scope_qualified;
+ }
+
+ protected:
+ virtual void Parse() {
+ m_parsed = true;
+ m_parse_error = true;
+ }
+
+ ConstString m_full; // Full name:
+ // "size_t lldb::SBTarget::GetBreakpointAtIndex(unsigned
+ // int) const"
+ llvm::StringRef m_basename; // Basename: "GetBreakpointAtIndex"
+ llvm::StringRef m_context; // Decl context: "lldb::SBTarget"
+ llvm::StringRef m_arguments; // Arguments: "(unsigned int)"
+ llvm::StringRef m_qualifiers; // Qualifiers: "const"
+ llvm::StringRef m_return_type; // Return type: "size_t"
+ std::string m_scope_qualified;
+ bool m_parsed = false;
+ bool m_parse_error = false;
+ };
+
+ virtual std::unique_ptr<Language::MethodName>
+ GetMethodName(ConstString name) const {
+ return std::make_unique<Language::MethodName>(name);
+ };
+
+ virtual std::pair<lldb::FunctionNameType, llvm::StringRef>
+ GetFunctionNameInfo(ConstString name) const {
+ return std::pair{lldb::eFunctionNameTypeNone, llvm::StringRef()};
+ };
+
/// Returns true iff the given symbol name is compatible with the mangling
/// scheme of this language.
///
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 0a08da0fec230..f09b451ac414d 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,8 +16,7 @@ if (LLDB_ENABLE_CURSES)
endif()
endif()
-# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
-add_lldb_library(lldbCore
+add_lldb_library(lldbCore NO_PLUGIN_DEPENDENCIES
Address.cpp
AddressRange.cpp
AddressRangeListImpl.cpp
@@ -71,8 +70,6 @@ add_lldb_library(lldbCore
lldbUtility
lldbValueObject
lldbVersion
- lldbPluginCPlusPlusLanguage
- lldbPluginObjCLanguage
${LLDB_CURSES_LIBS}
CLANG_LIBS
diff --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index ddaaedea04183..c41f0fdb5ff1b 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -33,12 +33,12 @@
#include <cstring>
using namespace lldb_private;
-static inline bool cstring_is_mangled(llvm::StringRef s) {
- return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
-}
-
#pragma mark Mangled
+bool Mangled::IsMangledName(llvm::StringRef name) {
+ return Mangled::GetManglingScheme(name) != Mangled::eManglingSchemeNone;
+}
+
Mangled::ManglingScheme Mangled::GetManglingScheme(llvm::StringRef const name) {
if (name.empty())
return Mangled::eManglingSchemeNone;
@@ -121,7 +121,7 @@ int Mangled::Compare(const Mangled &a, const Mangled &b) {
void Mangled::SetValue(ConstString name) {
if (name) {
- if (cstring_is_mangled(name.GetStringRef())) {
+ if (IsMangledName(name.GetStringRef())) {
m_demangled.Clear();
m_mangled = name;
} else {
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 53dc6fcde0381..8855d46396bdc 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -52,9 +52,6 @@
#include "lldb/Host/windows/PosixApi.h"
#endif
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
-
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/DJB.h"
@@ -641,98 +638,78 @@ void Module::FindCompileUnits(const FileSpec &path,
Module::LookupInfo::LookupInfo(ConstString name,
FunctionNameType name_type_mask,
LanguageType language)
- : m_name(name), m_lookup_name(), m_language(language) {
- const char *name_cstr = name.GetCString();
+ : m_name(name), m_lookup_name(name), m_language(language) {
llvm::StringRef basename;
- llvm::StringRef context;
+ std::vector<Language *> languages;
if (name_type_mask & eFunctionNameTypeAuto) {
- if (CPlusPlusLanguage::IsCPPMangledName(name_cstr))
- m_name_type_mask = eFunctionNameTypeFull;
- else if ((language == eLanguageTypeUnknown ||
- Language::LanguageIsObjC(language)) &&
- ObjCLanguage::IsPossibleObjCMethodName(name_cstr))
- m_name_type_mask = eFunctionNameTypeFull;
- else if (Language::LanguageIsC(language)) {
+ if (Mangled::IsMangledName(name_cstr) || Language::LanguageIsC(language))
m_name_type_mask = eFunctionNameTypeFull;
+ else if (language == eLanguageTypeUnknown) {
+ languages.push_back(Language::FindPlugin(eLanguageTypeObjC));
+ languages.push_back(Language::FindPlugin(eLanguageTypeC_plus_plus));
+ for (Language *lang : languages) {
+ if (!lang)
+ continue;
+ auto info = lang->GetFunctionNameInfo(name);
+ if (info.first != eFunctionNameTypeNone) {
+ m_name_type_mask = info.first;
+ basename = info.second;
+ break;
+ }
+ }
} else {
- if ((language == eLanguageTypeUnknown ||
- Language::LanguageIsObjC(language)) &&
- ObjCLanguage::IsPossibleObjCSelector(name_cstr))
- m_name_type_mask |= eFunctionNameTypeSelector;
-
- CPlusPlusLanguage::MethodName cpp_method(name);
- basename = cpp_method.GetBasename();
- if (basename.empty()) {
- if (CPlusPlusLanguage::ExtractContextAndIdentifier(name_cstr, context,
- basename))
- m_name_type_mask |= (eFunctionNameTypeMethod | eFunctionNameTypeBase);
- else
- m_name_type_mask |= eFunctionNameTypeFull;
- } else {
- m_name_type_mask |= (eFunctionNameTypeMethod | eFunctionNameTypeBase);
+ if (auto *lang = Language::FindPlugin(language)) {
+ auto info = lang->GetFunctionNameInfo(name);
+ m_name_type_mask = info.first;
+ basename = info.second;
}
}
+
+ // NOTE: There are several ways to get here, but this is a fallback path in
+ // case the above does not succeed at extracting any useful information from
+ // the loaded language plugins.
+ if (m_name_type_mask == eFunctionNameTypeNone)
+ m_name_type_mask = eFunctionNameTypeFull;
+
} else {
m_name_type_mask = name_type_mask;
- if (name_type_mask & eFunctionNameTypeMethod ||
- name_type_mask & eFunctionNameTypeBase) {
- // If they've asked for a CPP method or function name and it can't be
- // that, we don't even need to search for CPP methods or names.
- CPlusPlusLanguage::MethodName cpp_method(name);
- if (cpp_method.IsValid()) {
- basename = cpp_method.GetBasename();
-
- if (!cpp_method.GetQualifiers().empty()) {
- // There is a "const" or other qualifier following the end of the
- // function parens, this can't be a eFunctionNameTypeBase
- m_name_type_mask &= ~(eFunctionNameTypeBase);
- if (m_name_type_mask == eFunctionNameTypeNone)
- return;
+ if (language == eLanguageTypeUnknown) {
+ languages.push_back(Language::FindPlugin(eLanguageTypeObjC));
+ languages.push_back(Language::FindPlugin(eLanguageTypeC_plus_plus));
+ for (Language *lang : languages) {
+ if (!lang)
+ continue;
+ auto info = lang->GetFunctionNameInfo(name);
+ if (info.first & m_name_type_mask) {
+ m_name_type_mask &= info.first;
+ basename = info.second;
+ break;
}
- } else {
- // If the CPP method parser didn't manage to chop this up, try to fill
- // in the base name if we can. If a::b::c is passed in, we need to just
- // look up "c", and then we'll filter the result later.
- CPlusPlusLanguage::ExtractContextAndIdentifier(name_cstr, context,
- basename);
- }
- }
-
- if (name_type_mask & eFunctionNameTypeSelector) {
- if (!ObjCLanguage::IsPossibleObjCSelector(name_cstr)) {
- m_name_type_mask &= ~(eFunctionNameTypeSelector);
- if (m_name_type_mask == eFunctionNameTypeNone)
- return;
}
- }
-
- // Still try and get a basename in case someone specifies a name type mask
- // of eFunctionNameTypeFull and a name like "A::func"
- if (basename.empty()) {
- if (name_type_mask & eFunctionNameTypeFull &&
- !CPlusPlusLanguage::IsCPPMangledName(name_cstr)) {
- CPlusPlusLanguage::MethodName cpp_method(name);
- basename = cpp_method.GetBasename();
- if (basename.empty())
- CPlusPlusLanguage::ExtractContextAndIdentifier(name_cstr, context,
- basename);
+ } else {
+ if (auto *lang = Language::FindPlugin(language)) {
+ auto info = lang->GetFunctionNameInfo(name);
+ if (info.first & m_name_type_mask) {
+ // If the user asked for FunctionNameTypes that aren't possible,
+ // then filter those out. (e.g. asking for Selectors on
+ // C++ symbols, or even if the symbol given can't be a selector in
+ // ObjC)
+ m_name_type_mask &= info.first;
+ basename = info.second;
+ }
}
}
}
if (!basename.empty()) {
- // The name supplied was a partial C++ path like "a::count". In this case
- // we want to do a lookup on the basename "count" and then make sure any
- // matching results contain "a::count" so that it would match "b::a::count"
- // and "a::count". This is why we set "match_name_after_lookup" to true
+ // The name supplied was incomplete for lookup purposes. For example, in C++
+ // we may have gotten something like "a::count". In this case, we want to do
+ // a lookup on the basename "count" and then make sure any matching results
+ // contain "a::count" so that it would match "b::a::count" and "a::count".
+ // This is why we set match_name_after_lookup to true.
m_lookup_name.SetString(basename);
m_match_name_after_lookup = true;
- } else {
- // The name is already correct, just use the exact name as supplied, and we
- // won't need to check if any matches contain "name"
- m_lookup_name = name;
- m_match_name_after_lookup = false;
}
}
@@ -791,7 +768,8 @@ void Module::LookupInfo::Prune(SymbolContextList &sc_list,
// "func" and specified eFunctionNameTypeFull, but we might have found
// "a::func()", "a::b::func()", "c::func()", "func()" and "func". Only
// "func()" and "func" should end up matching.
- if (m_name_type_mask == eFunctionNameTypeFull) {
+ auto *lang = Language::FindPlugin(eLanguageTypeC_plus_plus);
+ if (lang && m_name_type_mask == eFunctionNameTypeFull) {
SymbolContext sc;
size_t i = start_idx;
while (i < sc_list.GetSize()) {
@@ -802,20 +780,21 @@ void Module::LookupInfo::Prune(SymbolContextList &sc_list,
ConstString mangled_name(sc.GetFunctionName(Mangled::ePreferMangled));
ConstString full_name(sc.GetFunctionName());
if (mangled_name != m_name && full_name != m_name) {
- CPlusPlusLanguage::MethodName cpp_method(full_name);
- if (cpp_method.IsValid()) {
- if (cpp_method.GetContext().empty()) {
- if (cpp_method.GetBasename().compare(m_name) != 0) {
+ std::unique_ptr<Language::MethodName> cpp_method =
+ lang->GetMethodName(full_name);
+ if (cpp_method->IsValid()) {
+ if (cpp_method->GetContext().empty()) {
+ if (cpp_method->GetBasename().compare(m_name) != 0) {
sc_list.RemoveContextAtIndex(i);
continue;
}
} else {
std::string qualified_name;
llvm::StringRef anon_prefix("(anonymous namespace)");
- if (cpp_method.GetContext() == anon_prefix)
- qualified_name = cpp_method.GetBasename().str();
+ if (cpp_method->GetContext() == anon_prefix)
+ qualified_name = cpp_method->GetBasename().str();
else
- qualified_name = cpp_method.GetScopeQualifiedName();
+ qualified_name = cpp_method->GetScopeQualifiedName();
if (qualified_name != m_name.GetCString()) {
sc_list.RemoveContextAtIndex(i);
continue;
@@ -988,8 +967,7 @@ DebuggersOwningModuleRequestingInterruption(Module &module) {
for (auto debugger_sp : requestors) {
if (!debugger_sp->InterruptRequested())
continue;
- if (debugger_sp->GetTargetList()
- .AnyTargetContainsModule(module))
+ if (debugger_sp->GetTargetList().AnyTargetContainsModule(module))
interruptors.push_back(debugger_sp);
}
return interruptors;
diff --git a/lldb/source/Core/RichManglingContext.cpp b/lldb/source/Core/RichManglingContext.cpp
index b68c9e11581b4..82582a5d675a9 100644
--- a/lldb/source/Core/RichManglingContext.cpp
+++ b/lldb/source/Core/RichManglingContext.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "lldb/Core/RichManglingContext.h"
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
#include "lldb/Utility/LLDBLog.h"
#include "llvm/ADT/StringRef.h"
@@ -24,9 +23,8 @@ RichManglingContext::~RichManglingContext() {
void RichManglingContext::ResetCxxMethodParser() {
// If we want to support parsers for other languages some day, we need a
// switch here to delete the correct parser type.
- if (m_cxx_method_parser.has_value()) {
+ if (m_cxx_method_parser) {
assert(m_provider == PluginCxxLanguage);
- delete get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser);
m_cxx_method_parser.reset();
}
}
@@ -58,8 +56,11 @@ bool RichManglingContext::FromItaniumName(ConstString mangled) {
}
bool RichManglingContext::FromCxxMethodName(ConstString demangled) {
+ auto *lang = Language::FindPlugin(eLanguageTypeC_plus_plus);
+ if (!lang)
+ return false;
ResetProvider(PluginCxxLanguage);
- m_cxx_method_parser = new CPlusPlusLanguage::MethodName(demangled);
+ m_cxx_method_parser = lang->GetMethodName(demangled);
return true;
}
@@ -70,8 +71,7 @@ bool RichManglingContext::IsCtorOrDtor() const {
return m_ipd.isCtorOrDtor();
case PluginCxxLanguage: {
// We can only check for destructors here.
- auto base_name =
- get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)->GetBasename();
+ auto base_name = m_cxx_method_parser->GetBasename();
return base_name.starts_with("~");
}
case None:
@@ -118,8 +118,7 @@ llvm::StringRef RichManglingContext::ParseFunctionBaseName() {
return processIPDStrResult(buf, n);
}
case PluginCxxLanguage:
- return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
- ->GetBasename();
+ return m_cxx_method_parser->GetBasename();
case None:
return {};
}
@@ -135,8 +134,7 @@ llvm::StringRef RichManglingContext::ParseFunctionDeclContextName() {
return processIPDStrResult(buf, n);
}
case PluginCxxLanguage:
- return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
- ->GetContext();
+ return m_cxx_method_parser->GetContext();
case None:
return {};
}
@@ -152,9 +150,7 @@ llvm::StringRef RichManglingContext::ParseFullName() {
return processIPDStrResult(buf, n);
}
case PluginCxxLanguage:
- return get<CPlusPlusLanguage::MethodName>(m_cxx_method_parser)
- ->GetFullName()
- .GetStringRef();
+ return m_cxx_method_parser->GetFullName().GetStringRef();
case None:
return {};
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 9e96f6557c7ba..667cb8a900459 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -18,6 +18,7 @@
#include "NameSearchContext.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/Core/Address.h"
+#include "lldb/Core/Mangled.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Expression/DiagnosticManager.h"
@@ -35,6 +36,7 @@
#include "lldb/Symbol/Variable.h"
#include "lldb/Symbol/VariableList.h"
#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Language.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/StackFrame.h"
...
[truncated]
|
75c47b9
to
d89b024
Compare
With the current path,
I'll try to find some time tomorrow to debug this, unless @Michael137 or @bulbazord have time before I get around to it. |
…-server dependencies The original PR is llvm#132274. Co-authored-by: @bulbazord Alex Langford
d89b024
to
5e365a8
Compare
Note
so cplusplus plug-in was called many times. The order of language plug-ins in CMakeLists.txt does not matter. I have optimized Note if name_type_mask = eFunctionNameTypeAuto it is very important to call all plugins and cplusplus->GetFunctionNameInfo(). gives The TestExternCSymbols.py test failed because of missing logic
It must be fixed now. |
With the last commit, I observe no test regressions on macOS (on my pretty default build config). |
Ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests pas on macOS. Thanks!
…-server dependencies (llvm#135033) The original PR is llvm#132274. Co-authored-by: @bulbazord Alex Langford
The original PR is #132274.
Co-authored-by: @bulbazord Alex Langford