I agree that refactor-as-you-touch-things is definitely the way to go, but if we're going to end up having a test/commands for all the command line tests, then we might as well start it sooner rather than later. On the other hand, if we dont' all agree that that's the best path forward, then I guess test/functionalities would be the place for it to go now, and we could continue that discussion on the other thread. The original therad is here <http://lists.llvm.org/pipermail/lldb-dev/2015-September/008224.html> if you want to chime in.
On Thu, Oct 1, 2015 at 2:17 PM Enrico Granata <egran...@apple.com> wrote: > On Oct 1, 2015, at 1:15 PM, Zachary Turner <ztur...@google.com> wrote: > > I see there's SBType, does this represent something different? > > > Yes, it represents types. > Type lookup does not necessarily always deal with types. > For instance, imagine a language that has function declarations such that > > function foo(int x, int y) returns int > > and > > function bar(int x, int y) returns int > > represents two different declarations - but they have the same type > > function (int, int) returns int > > We want type lookup to be able to maintain that distinction, such that one > could say > > (lldb) type lookup -l newlanguage — foo > *function foo(int x, int y) returns int* > > vs. > > (lldb) type lookup -l newlanguage — bar > *function bar(int x, int y) returns int* > > Other similar cases are languages where a runtime introspection mechanism > does not expose fully-baked types, but still provides useful introspection > information that one could show the user. Those are useful things for type > lookup to do, and forcing the results to be CompilerType*s* would lose us > that ability. We would need something like SBTypeOrDecl, or an opaque > SBTypeLookupResult. > > I was thinking you could make a method like > > SBType * > SBTarget::LookupType(const char* type_name, eLanguage lang); > > Even if it didn't support every possible usage of the "type lookup" > command, it would be a start. > > > I would like to avoid having a command that does the right thing always, > and an API that does somewhat a good thing sometimes. Note that I don’t say > this to oppose the notion of an API for “type lookup”, but only to oppose > the notion of an API that returns a list of types, when the output of “type > lookup” is not necessarily always just types. > > An API is a reasonable thing to ask for - I can’t promise I’ll have one > done today or tomorrow, especially because I have given no thought to how > to design one, but now that you mention it, it is in my queue of > improvements for this functionality. > > > At one point on the list it was discussed whether tests which use runCmd > and then matching the output should be separated from the rest of the test > suite, so there's a more clearly enforced distinction about which tests use > the command API and which tests use the python API. At least Jim thought > that was a good idea and supported the move, but nobody else really chimed > in. If you agree, maybe you could be the first? :) Something like > lldb/test/commands/type-lookup. Eventually the goal would just be to have > 3 top level directories, > > lldb > |__test > |__commands > |__python-api > |__unit-tests > > > I have historically been putting this kind of things in > “test/functionalities”. I am not sure that - as a matter of convention - we > have command-line only tests in “functionalities” though. > Is there any objection to a refactor as-you-touch-things approach here? > i.e. if I add a test for the command-line side, it goes in > test/functionalities/* - but if at a later time I add SB API for type > lookup, then the test for that goes in test/python-api/* > It seems a reasonable way forward to me - unless I am missing something > > > With the existing sub-trees divided up accordingly between the 3. (This > is all assuming of course that making the SBTarget::LookupType method I > suggested earlier doesn't work for some reason) > > On Thu, Oct 1, 2015 at 1:01 PM Enrico Granata <egran...@apple.com> wrote: > >> On Oct 1, 2015, at 12:49 PM, Zachary Turner <ztur...@google.com> wrote: >> >> Hi Enrico, >> >> Could you add some tests for this? >> >> >> Sure. I am still churning through a thing or two before declaring >> victory, but I can add tests as I make progress >> >> python API tests and appropriate methods added to SBTarget >> >> >> There currently is no SB API component to this. It is a purely >> command-line facility. >> If you feel it would be useful to have SB API for this, it will need to >> be designed (e.g. “type lookup” is allowed to transact in things that are >> not strictly speaking compiler types - we have no API way to represent this >> kind of concept) >> >> would be ideal. >> >> On Thu, Oct 1, 2015 at 11:17 AM Enrico Granata via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> >>> Author: enrico >>> Date: Thu Oct 1 13:16:18 2015 >>> New Revision: 249047 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=249047&view=rev >>> Log: >>> Add a 'type lookup' command. This command is meant to look up type >>> information by name in a language-specific way. >>> >>> Currently, it only supports Objective-C - C++ types can be looked up >>> through debug info via 'image lookup -t', whereas ObjC types via this >>> command are looked up by runtime introspection >>> >>> This behavior is in line with type lookup's behavior in Xcode 7, but I >>> am definitely open to feedback as to what makes the most sense here >>> >>> >>> Modified: >>> lldb/trunk/include/lldb/Target/Language.h >>> lldb/trunk/source/Commands/CommandObjectType.cpp >>> lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp >>> lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h >>> lldb/trunk/source/Target/Language.cpp >>> >>> Modified: lldb/trunk/include/lldb/Target/Language.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Language.h?rev=249047&r1=249046&r2=249047&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/include/lldb/Target/Language.h (original) >>> +++ lldb/trunk/include/lldb/Target/Language.h Thu Oct 1 13:16:18 2015 >>> @@ -13,6 +13,8 @@ >>> // C Includes >>> // C++ Includes >>> #include <functional> >>> +#include <memory> >>> +#include <set> >>> #include <vector> >>> >>> // Other libraries and framework includes >>> @@ -29,6 +31,42 @@ class Language : >>> public PluginInterface >>> { >>> public: >>> + >>> + class TypeScavenger >>> + { >>> + public: >>> + class Result >>> + { >>> + public: >>> + virtual bool >>> + IsValid () = 0; >>> + >>> + virtual bool >>> + DumpToStream (Stream& stream, >>> + bool print_help_if_available) = 0; >>> + >>> + virtual ~Result() = default; >>> + }; >>> + >>> + typedef std::set<std::unique_ptr<Result>> ResultSet; >>> + >>> + virtual ~TypeScavenger () = default; >>> + >>> + size_t >>> + Find (ExecutionContextScope *exe_scope, >>> + const char *key, >>> + ResultSet &results, >>> + bool append = true); >>> + >>> + protected: >>> + TypeScavenger () = default; >>> + >>> + virtual bool >>> + Find_Impl (ExecutionContextScope *exe_scope, >>> + const char *key, >>> + ResultSet &results) = 0; >>> + }; >>> + >>> ~Language() override; >>> >>> static Language* >>> @@ -65,6 +103,9 @@ public: >>> virtual lldb_private::formatters::StringPrinter::EscapingHelper >>> GetStringPrinterEscapingHelper >>> (lldb_private::formatters::StringPrinter::GetPrintableElementType); >>> >>> + virtual std::unique_ptr<TypeScavenger> >>> + GetTypeScavenger (); >>> + >>> // These are accessors for general information about the Languages >>> lldb knows about: >>> >>> static lldb::LanguageType >>> @@ -91,7 +132,6 @@ public: >>> >>> static bool >>> LanguageIsPascal (lldb::LanguageType language); >>> - >>> >>> protected: >>> //------------------------------------------------------------------ >>> >>> Modified: lldb/trunk/source/Commands/CommandObjectType.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectType.cpp?rev=249047&r1=249046&r2=249047&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Commands/CommandObjectType.cpp (original) >>> +++ lldb/trunk/source/Commands/CommandObjectType.cpp Thu Oct 1 13:16:18 >>> 2015 >>> @@ -4558,6 +4558,218 @@ CommandObjectTypeFilterAdd::CommandOptio >>> { 0, false, NULL, 0, 0, NULL, NULL, 0, eArgTypeNone, NULL } >>> }; >>> >>> +//---------------------------------------------------------------------- >>> +// "type lookup" >>> +//---------------------------------------------------------------------- >>> +class CommandObjectTypeLookup : public CommandObjectRaw >>> +{ >>> +protected: >>> + >>> + class CommandOptions : public OptionGroup >>> + { >>> + public: >>> + >>> + CommandOptions () : >>> + OptionGroup(), >>> + m_show_help(false), >>> + m_language(eLanguageTypeUnknown) >>> + {} >>> + >>> + virtual >>> + ~CommandOptions () {} >>> + >>> + virtual uint32_t >>> + GetNumDefinitions () >>> + { >>> + return 3; >>> + } >>> + >>> + virtual const OptionDefinition* >>> + GetDefinitions () >>> + { >>> + return g_option_table; >>> + } >>> + >>> + virtual Error >>> + SetOptionValue (CommandInterpreter &interpreter, >>> + uint32_t option_idx, >>> + const char *option_value) >>> + { >>> + Error error; >>> + >>> + const int short_option = >>> g_option_table[option_idx].short_option; >>> + >>> + switch (short_option) >>> + { >>> + case 'h': >>> + m_show_help = true; >>> + break; >>> + >>> + case 'l': >>> + m_language = >>> Language::GetLanguageTypeFromString(option_value); >>> + break; >>> + >>> + default: >>> + error.SetErrorStringWithFormat("invalid short >>> option character '%c'", short_option); >>> + break; >>> + } >>> + >>> + return error; >>> + } >>> + >>> + virtual void >>> + OptionParsingStarting (CommandInterpreter &interpreter) >>> + { >>> + m_show_help = false; >>> + m_language = eLanguageTypeUnknown; >>> + } >>> + >>> + // Options table: Required for subclasses of Options. >>> + >>> + static OptionDefinition g_option_table[]; >>> + bool m_show_help; >>> + lldb::LanguageType m_language; >>> + }; >>> + >>> + OptionGroupOptions m_option_group; >>> + CommandOptions m_command_options; >>> + >>> +public: >>> + >>> + CommandObjectTypeLookup (CommandInterpreter &interpreter) : >>> + CommandObjectRaw (interpreter, >>> + "type lookup", >>> + "Lookup a type by name in the select target.", >>> + "type lookup <typename>", >>> + eCommandRequiresTarget), >>> + m_option_group(interpreter), >>> + m_command_options() >>> + { >>> + m_option_group.Append(&m_command_options); >>> + m_option_group.Finalize(); >>> + } >>> + >>> + virtual >>> + ~CommandObjectTypeLookup () >>> + { >>> + } >>> + >>> + virtual >>> + Options * >>> + GetOptions () >>> + { >>> + return &m_option_group; >>> + } >>> + >>> + virtual bool >>> + DoExecute (const char *raw_command_line, CommandReturnObject >>> &result) >>> + { >>> + if (!raw_command_line || !raw_command_line[0]) >>> + { >>> + result.SetError("type lookup cannot be invoked without a >>> type name as argument"); >>> + return false; >>> + } >>> + >>> + m_option_group.NotifyOptionParsingStarting(); >>> + >>> + const char * name_of_type = NULL; >>> + >>> + if (raw_command_line[0] == '-') >>> + { >>> + // We have some options and these options MUST end with --. >>> + const char *end_options = NULL; >>> + const char *s = raw_command_line; >>> + while (s && s[0]) >>> + { >>> + end_options = ::strstr (s, "--"); >>> + if (end_options) >>> + { >>> + end_options += 2; // Get past the "--" >>> + if (::isspace (end_options[0])) >>> + { >>> + name_of_type = end_options; >>> + while (::isspace (*name_of_type)) >>> + ++name_of_type; >>> + break; >>> + } >>> + } >>> + s = end_options; >>> + } >>> + >>> + if (end_options) >>> + { >>> + Args args (llvm::StringRef(raw_command_line, >>> end_options - raw_command_line)); >>> + if (!ParseOptions (args, result)) >>> + return false; >>> + >>> + Error error >>> (m_option_group.NotifyOptionParsingFinished()); >>> + if (error.Fail()) >>> + { >>> + result.AppendError (error.AsCString()); >>> + result.SetStatus (eReturnStatusFailed); >>> + return false; >>> + } >>> + } >>> + } >>> + if (nullptr == name_of_type) >>> + name_of_type = raw_command_line; >>> + >>> + TargetSP >>> target_sp(GetCommandInterpreter().GetDebugger().GetSelectedTarget()); >>> + const bool fill_all_in = true; >>> + ExecutionContext exe_ctx(target_sp.get(), fill_all_in); >>> + ExecutionContextScope *best_scope = >>> exe_ctx.GetBestExecutionContextScope(); >>> + >>> + bool any_found = false; >>> + >>> + std::vector<Language*> languages; >>> + >>> + if (m_command_options.m_language == eLanguageTypeUnknown) >>> + { >>> + // FIXME: hardcoding languages is not good >>> + >>> languages.push_back(Language::FindPlugin(eLanguageTypeObjC)); >>> + >>> languages.push_back(Language::FindPlugin(eLanguageTypeC_plus_plus)); >>> + } >>> + else >>> + { >>> + >>> languages.push_back(Language::FindPlugin(m_command_options.m_language)); >>> + } >>> + >>> + for (Language* language : languages) >>> + { >>> + if (!language) >>> + continue; >>> + >>> + if (auto scavenger = language->GetTypeScavenger()) >>> + { >>> + Language::TypeScavenger::ResultSet search_results; >>> + if (scavenger->Find(best_scope, name_of_type, >>> search_results) > 0) >>> + { >>> + for (const auto& search_result : search_results) >>> + { >>> + if (search_result && search_result->IsValid()) >>> + { >>> + any_found = true; >>> + >>> search_result->DumpToStream(result.GetOutputStream(), >>> this->m_command_options.m_show_help); >>> + } >>> + } >>> + } >>> + } >>> + } >>> + >>> + result.SetStatus (any_found ? >>> lldb::eReturnStatusSuccessFinishResult : >>> lldb::eReturnStatusSuccessFinishNoResult); >>> + return true; >>> + } >>> + >>> +}; >>> + >>> +OptionDefinition >>> +CommandObjectTypeLookup::CommandOptions::g_option_table[] = >>> +{ >>> + { LLDB_OPT_SET_ALL, false, "show-help", 'h', >>> OptionParser::eNoArgument, NULL, NULL, 0, eArgTypeNone, "Display >>> available help for types"}, >>> + { LLDB_OPT_SET_ALL, false, "language", 'l', >>> OptionParser::eRequiredArgument, NULL, NULL, 0, eArgTypeLanguage, "Which >>> language's types should the search scope be"}, >>> + { 0, false, NULL, 0, 0, NULL, NULL, 0, eArgTypeNone, NULL } >>> +}; >>> + >>> template <typename FormatterType> >>> class CommandObjectFormatterInfo : public CommandObjectRaw >>> { >>> @@ -4778,6 +4990,7 @@ CommandObjectType::CommandObjectType (Co >>> #ifndef LLDB_DISABLE_PYTHON >>> LoadSubCommand ("synthetic", CommandObjectSP (new >>> CommandObjectTypeSynth (interpreter))); >>> #endif >>> + LoadSubCommand ("lookup", CommandObjectSP (new >>> CommandObjectTypeLookup (interpreter))); >>> } >>> >>> >>> >>> Modified: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp?rev=249047&r1=249046&r2=249047&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp (original) >>> +++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp Thu Oct 1 >>> 13:16:18 2015 >>> @@ -656,3 +656,91 @@ ObjCLanguage::GetPossibleFormattersMatch >>> >>> return result; >>> } >>> + >>> +std::unique_ptr<Language::TypeScavenger> >>> +ObjCLanguage::GetTypeScavenger () >>> +{ >>> + class ObjCTypeScavenger : public Language::TypeScavenger >>> + { >>> + private: >>> + class ObjCScavengerResult : public >>> Language::TypeScavenger::Result >>> + { >>> + public: >>> + ObjCScavengerResult (CompilerType type) : >>> + Language::TypeScavenger::Result(), >>> + m_compiler_type(type) >>> + { >>> + } >>> + >>> + bool >>> + IsValid () override >>> + { >>> + return m_compiler_type.IsValid(); >>> + } >>> + >>> + bool >>> + DumpToStream (Stream& stream, >>> + bool print_help_if_available) override >>> + { >>> + if (IsValid()) >>> + { >>> + m_compiler_type.DumpTypeDescription(&stream); >>> + stream.EOL(); >>> + return true; >>> + } >>> + return false; >>> + } >>> + >>> + virtual ~ObjCScavengerResult() = default; >>> + private: >>> + CompilerType m_compiler_type; >>> + }; >>> + >>> + protected: >>> + ObjCTypeScavenger() = default; >>> + >>> + virtual ~ObjCTypeScavenger() = default; >>> + >>> + bool >>> + Find_Impl (ExecutionContextScope *exe_scope, >>> + const char *key, >>> + ResultSet &results) override >>> + { >>> + bool result = false; >>> + >>> + Process* process = exe_scope->CalculateProcess().get(); >>> + if (process) >>> + { >>> + const bool create_on_demand = false; >>> + auto objc_runtime = >>> process->GetObjCLanguageRuntime(create_on_demand); >>> + if (objc_runtime) >>> + { >>> + auto decl_vendor = objc_runtime->GetDeclVendor(); >>> + if (decl_vendor) >>> + { >>> + std::vector<clang::NamedDecl *> decls; >>> + ConstString name(key); >>> + decl_vendor->FindDecls(name, true, UINT32_MAX, >>> decls); >>> + for (auto decl : decls) >>> + { >>> + if (decl) >>> + { >>> + if (CompilerType candidate = >>> ClangASTContext::GetTypeForDecl(decl)) >>> + { >>> + result = true; >>> + >>> std::unique_ptr<Language::TypeScavenger::Result> result(new >>> ObjCScavengerResult(candidate)); >>> + results.insert(std::move(result)); >>> + } >>> + } >>> + } >>> + } >>> + } >>> + } >>> + return result; >>> + } >>> + >>> + friend class ObjCLanguage; >>> + }; >>> + >>> + return std::unique_ptr<TypeScavenger>(new ObjCTypeScavenger()); >>> +} >>> >>> Modified: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h?rev=249047&r1=249046&r2=249047&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h (original) >>> +++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.h Thu Oct 1 >>> 13:16:18 2015 >>> @@ -146,6 +146,9 @@ public: >>> std::vector<ConstString> >>> GetPossibleFormattersMatches (ValueObject& valobj, >>> lldb::DynamicValueType use_dynamic) override; >>> >>> + std::unique_ptr<TypeScavenger> >>> + GetTypeScavenger () override; >>> + >>> //------------------------------------------------------------------ >>> // Static Functions >>> //------------------------------------------------------------------ >>> >>> Modified: lldb/trunk/source/Target/Language.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Language.cpp?rev=249047&r1=249046&r2=249047&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/Target/Language.cpp (original) >>> +++ lldb/trunk/source/Target/Language.cpp Thu Oct 1 13:16:18 2015 >>> @@ -287,6 +287,34 @@ Language::LanguageIsPascal (LanguageType >>> } >>> } >>> >>> +std::unique_ptr<Language::TypeScavenger> >>> +Language::GetTypeScavenger () >>> +{ >>> + return nullptr; >>> +} >>> + >>> +size_t >>> +Language::TypeScavenger::Find (ExecutionContextScope *exe_scope, >>> + const char *key, >>> + ResultSet &results, >>> + bool append) >>> +{ >>> + if (!exe_scope || !exe_scope->CalculateTarget().get()) >>> + return false; >>> + >>> + if (!key || !key[0]) >>> + return false; >>> + >>> + if (!append) >>> + results.clear(); >>> + >>> + size_t old_size = results.size(); >>> + >>> + if (this->Find_Impl(exe_scope, key, results)) >>> + return results.size() - old_size; >>> + return 0; >>> +} >>> + >>> //---------------------------------------------------------------------- >>> // Constructor >>> //---------------------------------------------------------------------- >>> >>> >>> _______________________________________________ >>> lldb-commits mailing list >>> lldb-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>> >> >> >> Thanks, >> *- Enrico* >> 📩 egranata@.com ☎️ 27683 >> >> > > Thanks, > *- Enrico* > 📩 egranata@.com ☎️ 27683 > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
