-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[lldb] Refactor LookupInfo object to be per-language #168797
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
Changes from all commits
8b04ffa
e55e378
1567174
c11b5d8
15edf30
78d4544
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -315,6 +315,15 @@ class Module : public std::enable_shared_from_this<Module>, | |
| const ModuleFunctionSearchOptions &options, | ||
| SymbolContextList &sc_list); | ||
|
|
||
| /// Find functions by a vector of lookup infos. | ||
| /// | ||
| /// If the function is an inlined function, it will have a block, | ||
| /// representing the inlined function, and the function will be the | ||
| /// containing function. If it is not inlined, then the block will be NULL. | ||
| void FindFunctions(const std::vector<LookupInfo> &lookup_infos, | ||
| const CompilerDeclContext &parent_decl_ctx, | ||
| const ModuleFunctionSearchOptions &options, | ||
| SymbolContextList &sc_list); | ||
| /// Find functions by name. | ||
| /// | ||
| /// If the function is an inlined function, it will have a block, | ||
|
|
@@ -917,8 +926,29 @@ class Module : public std::enable_shared_from_this<Module>, | |
| public: | ||
| LookupInfo() = default; | ||
|
|
||
| LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask, | ||
| lldb::LanguageType language); | ||
| /// Creates a vector of lookup infos for function name resolution. | ||
| /// | ||
| /// \param[in] name | ||
| /// The function name to search for. This can be a simple name like | ||
| /// "foo" or a qualified name like "Class::method". | ||
| /// | ||
| /// \param[in] name_type_mask | ||
| /// A bitmask specifying what types of names to search for | ||
| /// (e.g., eFunctionNameTypeFull, eFunctionNameTypeBase, | ||
| /// eFunctionNameTypeMethod, eFunctionNameTypeAuto). Multiple types | ||
| /// can be combined with bitwise OR. | ||
| /// | ||
| /// \param[in] lang_type | ||
| /// The language to create lookups for. If eLanguageTypeUnknown is | ||
| /// passed, creates one LookupInfo for each language plugin currently | ||
| /// available in LLDB. If a specific language is provided, creates only | ||
| // a single LookupInfo for that language. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know I am a bit late to the party, but I think this is odd from an interface point of view. We could probably have two overloads: one which takes a LanguageType, and returns a single LookupInfo, and one which doesn't take a LanguageType, and returns a vector of LookupInfo. As it stands, we are making all clients of this function handle vector types, which feels wrong, as it creates more code paths to check for (e.g. check that the vector is not empty, and check that the vector contains a single element). By having the specialized signature, we define all these runtime checks away.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the existence of the "unknown enum" makes it annoying to have this different API.... :( |
||
| /// | ||
| /// \return | ||
| /// A vector of LookupInfo objects, one per relevant language. | ||
| static std::vector<LookupInfo> | ||
| MakeLookupInfos(ConstString name, lldb::FunctionNameType name_type_mask, | ||
| lldb::LanguageType lang_type); | ||
|
|
||
| ConstString GetName() const { return m_name; } | ||
|
|
||
|
|
@@ -959,6 +989,10 @@ class Module : public std::enable_shared_from_this<Module>, | |
| /// If \b true, then demangled names that match will need to contain | ||
| /// "m_name" in order to be considered a match | ||
| bool m_match_name_after_lookup = false; | ||
|
|
||
| private: | ||
| LookupInfo(ConstString name, lldb::FunctionNameType name_type_mask, | ||
| lldb::LanguageType lang_type); | ||
| }; | ||
|
|
||
| /// Get a unique hash for this module. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -643,26 +643,13 @@ void Module::FindCompileUnits(const FileSpec &path, | |
|
|
||
| Module::LookupInfo::LookupInfo(ConstString name, | ||
| FunctionNameType name_type_mask, | ||
| LanguageType language) | ||
| : m_name(name), m_lookup_name(name), m_language(language) { | ||
| LanguageType lang_type) | ||
| : m_name(name), m_lookup_name(name), m_language(lang_type) { | ||
| std::optional<ConstString> basename; | ||
|
|
||
| std::vector<Language *> languages; | ||
| { | ||
| std::vector<LanguageType> lang_types; | ||
| if (language != eLanguageTypeUnknown) | ||
| lang_types.push_back(language); | ||
| else | ||
| lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus}; | ||
|
|
||
| for (LanguageType lang_type : lang_types) { | ||
| if (Language *lang = Language::FindPlugin(lang_type)) | ||
| languages.push_back(lang); | ||
| } | ||
| } | ||
| Language *lang = Language::FindPlugin(lang_type); | ||
|
|
||
| if (name_type_mask & eFunctionNameTypeAuto) { | ||
| for (Language *lang : languages) { | ||
| if (lang) { | ||
| auto info = lang->GetFunctionNameInfo(name); | ||
| if (info.first != eFunctionNameTypeNone) { | ||
| m_name_type_mask |= info.first; | ||
|
|
@@ -679,7 +666,7 @@ Module::LookupInfo::LookupInfo(ConstString name, | |
|
|
||
| } else { | ||
| m_name_type_mask = name_type_mask; | ||
| for (Language *lang : languages) { | ||
| if (lang) { | ||
| auto info = lang->GetFunctionNameInfo(name); | ||
| if (info.first & m_name_type_mask) { | ||
| // If the user asked for FunctionNameTypes that aren't possible, | ||
|
|
@@ -688,14 +675,12 @@ Module::LookupInfo::LookupInfo(ConstString name, | |
| // ObjC) | ||
| m_name_type_mask &= info.first; | ||
| basename = info.second; | ||
| break; | ||
| } | ||
| // Still try and get a basename in case someone specifies a name type mask | ||
| // of eFunctionNameTypeFull and a name like "A::func" | ||
| if (name_type_mask & eFunctionNameTypeFull && | ||
| info.first != eFunctionNameTypeNone && !basename && info.second) { | ||
| } else if (name_type_mask & eFunctionNameTypeFull && | ||
| info.first != eFunctionNameTypeNone && !basename && | ||
| info.second) { | ||
| // Still try and get a basename in case someone specifies a name type | ||
| // mask of eFunctionNameTypeFull and a name like "A::func" | ||
| basename = info.second; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -711,6 +696,36 @@ Module::LookupInfo::LookupInfo(ConstString name, | |
| } | ||
| } | ||
|
|
||
| std::vector<Module::LookupInfo> | ||
| Module::LookupInfo::MakeLookupInfos(ConstString name, | ||
| lldb::FunctionNameType name_type_mask, | ||
| lldb::LanguageType lang_type) { | ||
| std::vector<LanguageType> lang_types; | ||
| if (lang_type != eLanguageTypeUnknown) { | ||
| lang_types.push_back(lang_type); | ||
| } else { | ||
| // If the language type was not specified, look up in every language | ||
| // available. | ||
| Language::ForEach([&](Language *lang) { | ||
| auto lang_type = lang->GetLanguageType(); | ||
| if (!llvm::is_contained(lang_types, lang_type)) | ||
| lang_types.push_back(lang_type); | ||
| return IterationAction::Continue; | ||
| }); | ||
|
|
||
| if (lang_types.empty()) | ||
| lang_types = {eLanguageTypeObjC, eLanguageTypeC_plus_plus}; | ||
| } | ||
|
|
||
| std::vector<Module::LookupInfo> infos; | ||
| infos.reserve(lang_types.size()); | ||
| for (LanguageType lang_type : lang_types) { | ||
| Module::LookupInfo info(name, name_type_mask, lang_type); | ||
| infos.push_back(info); | ||
|
Comment on lines
+723
to
+724
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm apparently we can't because LookupInfo's constructor is private so vector can't access it. |
||
| } | ||
| return infos; | ||
| } | ||
|
|
||
| bool Module::LookupInfo::NameMatchesLookupInfo( | ||
| ConstString function_name, LanguageType language_type) const { | ||
| // We always keep unnamed symbols | ||
|
|
@@ -824,18 +839,29 @@ void Module::FindFunctions(const Module::LookupInfo &lookup_info, | |
| } | ||
| } | ||
|
|
||
| void Module::FindFunctions(const std::vector<Module::LookupInfo> &lookup_infos, | ||
| const CompilerDeclContext &parent_decl_ctx, | ||
| const ModuleFunctionSearchOptions &options, | ||
| SymbolContextList &sc_list) { | ||
| for (auto &lookup_info : lookup_infos) | ||
| FindFunctions(lookup_info, parent_decl_ctx, options, sc_list); | ||
| } | ||
|
|
||
| void Module::FindFunctions(ConstString name, | ||
| const CompilerDeclContext &parent_decl_ctx, | ||
| FunctionNameType name_type_mask, | ||
| const ModuleFunctionSearchOptions &options, | ||
| SymbolContextList &sc_list) { | ||
| const size_t old_size = sc_list.GetSize(); | ||
| LookupInfo lookup_info(name, name_type_mask, eLanguageTypeUnknown); | ||
| FindFunctions(lookup_info, parent_decl_ctx, options, sc_list); | ||
| if (name_type_mask & eFunctionNameTypeAuto) { | ||
| const size_t new_size = sc_list.GetSize(); | ||
| if (old_size < new_size) | ||
| lookup_info.Prune(sc_list, old_size); | ||
| std::vector<LookupInfo> lookup_infos = | ||
| LookupInfo::MakeLookupInfos(name, name_type_mask, eLanguageTypeUnknown); | ||
| for (auto &lookup_info : lookup_infos) { | ||
| const size_t old_size = sc_list.GetSize(); | ||
| FindFunctions(lookup_info, parent_decl_ctx, options, sc_list); | ||
| if (name_type_mask & eFunctionNameTypeAuto) { | ||
| const size_t new_size = sc_list.GetSize(); | ||
| if (old_size < new_size) | ||
| lookup_info.Prune(sc_list, old_size); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
we should never use
const vector&as a parameter inside LLVM, favouringllvm::ArrayRefinstead, as it works with more kinds of containers and, more importantly, it works with a single element of the value type.