clayborg added inline comments.
================ Comment at: lldb/include/lldb/Core/Module.h:435 + /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a". + lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match); ---------------- aprantl wrote: > Why is this not taking a TypeQuery that wraps a name? This function doesn't need to exist as long as the "TypeSP TypeQuery::FindFirstType(Module)" function is ok to have around. ================ Comment at: lldb/include/lldb/Core/Module.h:442 /// - /// \param searched_symbol_files - /// Prevents one file from being visited multiple times. - void - FindTypes(llvm::ArrayRef<CompilerContext> pattern, LanguageSet languages, - llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files, - TypeMap &types); - - lldb::TypeSP FindFirstType(const SymbolContext &sc, ConstString type_name, - bool exact_match); - - /// Find types by name that are in a namespace. This function is used by the - /// expression parser when searches need to happen in an exact namespace - /// scope. + /// \param[in] search_first + /// If non-null, this module will be searched before any other ---------------- aprantl wrote: > copy&paste error? yep! ================ Comment at: lldb/include/lldb/Core/ModuleList.h:373 + /// match: "b::a", "c::b::a", "d::b::a", "e::f::b::a". + lldb::TypeSP FindFirstType(Module *search_first, ConstString type_name, + bool exact_match) const; ---------------- aprantl wrote: > same question — why is this not a TypeQuery? Like in Module.h we can get rid of this as long as the convenience functions in TypeQuery::FindFirstType() are ok. ================ Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:90 + /// \param context A valid vector of CompilerContext entries that describes + /// this delcaratiion context. The first entry in the vector is the parent of + /// the subsequent entry, so the top most entry is the global namespace. ---------------- aprantl wrote: > declaration will fix ================ Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93 + void GetCompilerContext( + llvm::SmallVectorImpl<lldb_private::CompilerContext> &context) const; + ---------------- aprantl wrote: > aprantl wrote: > > Why can't this be a return value? The context objects are tiny. > ping? Will change! ================ Comment at: lldb/include/lldb/Symbol/Type.h:85 + /// doing name lookups. + TypeQuery() = default; + ---------------- aprantl wrote: > Can we get rid of this now? I believe so! I will change "default" to "delete". ================ Comment at: lldb/include/lldb/Symbol/Type.h:241 + /// Add a language family to the list of languages that should produce a match. + void AddLanguage(lldb::LanguageType language); + ---------------- aprantl wrote: > Is this necessary, or could this be rolled into the constructor? I will add it to the constructor. ================ Comment at: lldb/include/lldb/Symbol/Type.h:306 + /// more complete are are used when lookup up types in a clang module's debug + /// information. + bool m_module_search = false; ---------------- aprantl wrote: > This sentence is missing at least one word. (I also don't quite understand > the purpose of this flag) I will reword Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137900/new/ https://reviews.llvm.org/D137900 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
