> 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 CompilerTypes 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 
> <mailto:egran...@apple.com>> wrote:
>> On Oct 1, 2015, at 12:49 PM, Zachary Turner <ztur...@google.com 
>> <mailto: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 <mailto: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 
>> <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
>>  
>> <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
>>  
>> <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
>>  
>> <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
>>  
>> <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
>>  
>> <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 <mailto:lldb-commits@lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
>> <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

Reply via email to