[Lldb-commits] [lldb] r247082 - Implement a Target::GetTypeSystemForLanguage API, as well as provide helpers on the TypeSystem to get numeric types of specific sizes and signedness

Enrico Granata via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 10 11:26:03 PDT 2015


> On Sep 9, 2015, at 3:58 PM, Ryan Brown <ribrdb at google.com> wrote:
> 
> 
> 
> On Wed, Sep 9, 2015 at 2:57 PM Enrico Granata <egranata at apple.com <mailto:egranata at apple.com>> wrote:
>> On Sep 9, 2015, at 2:47 PM, Ryan Brown <ribrdb at google.com <mailto:ribrdb at google.com>> wrote:
>> 
>> 
>> On Tue, Sep 8, 2015 at 3:10 PM Enrico Granata via lldb-commits <lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>> wrote:
>> Author: enrico
>> Date: Tue Sep  8 17:09:19 2015
>> New Revision: 247082
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=247082&view=rev <http://llvm.org/viewvc/llvm-project?rev=247082&view=rev>
>> Log:
>> Implement a Target::GetTypeSystemForLanguage API, as well as provide helpers on the TypeSystem to get numeric types of specific sizes and signedness
>> 
>> Modified:
>>     lldb/trunk/include/lldb/Symbol/ClangASTContext.h
>>     lldb/trunk/include/lldb/Symbol/TypeSystem.h
>>     lldb/trunk/include/lldb/Target/Target.h
>>     lldb/trunk/source/DataFormatters/CoreMedia.cpp
>>     lldb/trunk/source/DataFormatters/VectorType.cpp
>>     lldb/trunk/source/Target/Target.cpp
>> 
>> Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
>> +++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Tue Sep  8 17:09:19 2015
>> @@ -454,7 +454,7 @@ public:
>>      //------------------------------------------------------------------
>> 
>>      CompilerType
>> -    GetIntTypeFromBitSize (size_t bit_size, bool is_signed)
>> +    GetIntTypeFromBitSize (size_t bit_size, bool is_signed) override
>>      {
>>          return GetIntTypeFromBitSize (getASTContext(), bit_size, is_signed);
>>      }
>> @@ -477,7 +477,7 @@ public:
>>      //------------------------------------------------------------------
>> 
>>      CompilerType
>> -    GetFloatTypeFromBitSize (size_t bit_size)
>> +    GetFloatTypeFromBitSize (size_t bit_size) override
>>      {
>>          return GetFloatTypeFromBitSize (getASTContext(), bit_size);
>>      }
>> 
>> Modified: lldb/trunk/include/lldb/Symbol/TypeSystem.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeSystem.h?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeSystem.h?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Symbol/TypeSystem.h (original)
>> +++ lldb/trunk/include/lldb/Symbol/TypeSystem.h Tue Sep  8 17:09:19 2015
>> @@ -400,6 +400,12 @@ public:
>>      virtual CompilerType
>>      GetBasicTypeFromAST (lldb::BasicType basic_type) = 0;
>> 
>> +    virtual CompilerType
>> +    GetIntTypeFromBitSize (size_t bit_size, bool is_signed) = 0;
>> +
>> +    virtual CompilerType
>> +    GetFloatTypeFromBitSize (size_t bit_size) = 0;
>> +
>> 
>> Why do these need to be here?
> 
> They are convenient to have when you are not transacting in terms of “int” or “float” but in terms of “I want a type that can represent a 32-bit unsigned integer value"
> 
>> It seems like everything using these is clang specific.
> 
> That might be because at the moment open-source LLDB does not have fully-baked type systems for non-clang languages
> I suspect that most languages have a notion of numeric types of a certain bit-size and signedness, so the API itself is not clang-specific whatsoever
> 
> I ask because I'm trying to add another TypeSystem.
> I could implement these, but I don't really like adding functionality that  isn't used and that I don't really have any way to test.
>  

Returning a nullptr TypeSystem and hollow CompilerTypes is fine if you’re not going to make any use of these

>>      virtual bool
>>      IsBeingDefined (void *type) = 0;
>> 
>> 
>> Modified: lldb/trunk/include/lldb/Target/Target.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Target/Target.h (original)
>> +++ lldb/trunk/include/lldb/Target/Target.h Tue Sep  8 17:09:19 2015
>> @@ -1230,6 +1230,14 @@ public:
>>      ClangASTContext *
>>      GetScratchClangASTContext(bool create_on_demand=true);
>> 
>> +    TypeSystem*
>> +    GetTypeSystemForLanguage (lldb::LanguageType language);
>> +
>> +    CompilerType
>> +    GetBasicType (lldb::LanguageType language,
>> +                  lldb::BasicType basic_type,
>> +                  size_t size = 0);
>> +
> 
> I am not sure the latter API (Target::GetBasicType) even needs to be there - my bad for checking that in
> The former one (Target::GetTypeSystemForLanguage) is what I truly meant to introduce
> 
>> 
>> Again, I'm not sure why this needs to be here when all uses are clang specific.
> 
> Same argument, the API has nothing clang-specific about it. The existing implementation/users are, but that is to be expected since clang is currently the only compiler that llvm.org <http://llvm.org/> LLDB is integrated with
> 
>> If it needs to be here, can you add some documentation?
> 
> Sure thing. Do you have any suggestions as to how to phrase it?
> 
>> When writing a new TypeSystem, does it need to be added to GetTypeSystemForLanguage?
> 
> Seems about right
> 
>> How would it be used? Do types from this "scratch" type system need to interact with types from a real, symbol file backed type system? 
> 
> Define “interact"
> Well for example in VectorType you use a CompilerType from the "scratch" type system with a ValueObject that might be using another type system. That seems strange to me.
> 

Truth be told, I am currently only classifying as “vector type” types coming from clang
If your new type system has a notion of vector type, then we can look at how to adjust the data formatter to work in both scenarios. I suspect that might be as simple as you returning proper types from your type system, and me using the ValueObject’s “minimum runtime language” as the one to ask for the scratch AST context instead of defaulting to C

>> What if the sizes of basic types aren't standardized and need to be lookup up in the debug info?
> 
> I assume this remark is for Target::GetBasicType(), and thus a moot point since I didn’t mean to check that declaration at all, and will remove it soon
> 
> It's not really moot because I need to know how to implement GetBasicType for the TypeSytem I return from Target::GetTypeSystemForLanguage(eLanguageTypeGo).
> I guess I'm wondering what the contract is for the TypeSystem I return.
> If only the GetXTypeFromBitSize methods are called things should work fine. But if someone calls GetBasicType(eBasicTypeInt), I can only guess what the compiler chose.

That depends on the semantics of your language. Apparently, for Go, “int” is a type alias for either 32 or 64 bit integers
I am not familiar with how the choice is made (pointer size, maybe?) so I can’t help you decide how to disambiguate
If some cases of GetBasicType() are undefined/non-existant in Go, returning an empty type is the right thing to do. For instance, if asked for an “id”, I would not expect Go to return anything sensible.
On the other hand, if Go has, say, an abstract base channel type, it would make sense for it to be a BasicType, where Go would respond correctly, but clang would just return an empty type.
If we need to adjust the vector formatter (or other formatters) such that they only ask for specific byte-sizes, we can work on that.

> 
> It seems like everything currently using GetScratchClangASTContext is inside language specific data formatters or runtimes, and I'm having a hard time imaging a use case that isn't language specific.
> 
> 
>> 
>> 
>>  
>>      ClangASTImporter *
>>      GetClangASTImporter();
>> 
>> 
>> Modified: lldb/trunk/source/DataFormatters/CoreMedia.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/CoreMedia.cpp?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/CoreMedia.cpp?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/source/DataFormatters/CoreMedia.cpp (original)
>> +++ lldb/trunk/source/DataFormatters/CoreMedia.cpp Tue Sep  8 17:09:19 2015
>> @@ -10,7 +10,7 @@
>>  #include "lldb/DataFormatters/CoreMedia.h"
>> 
>>  #include "lldb/Core/Flags.h"
>> -#include "lldb/Symbol/ClangASTContext.h"
>> +#include "lldb/Symbol/TypeSystem.h"
>>  #include "lldb/Target/Target.h"
>>  #include <inttypes.h>
>> 
>> @@ -25,13 +25,13 @@ lldb_private::formatters::CMTimeSummaryP
>>      if (!type.IsValid())
>>          return false;
>> 
>> -    ClangASTContext *ast_ctx = valobj.GetExecutionContextRef().GetTargetSP()->GetScratchClangASTContext();
>> -    if (!ast_ctx)
>> +    TypeSystem *type_system = valobj.GetExecutionContextRef().GetTargetSP()->GetTypeSystemForLanguage(lldb::eLanguageTypeC);
>> +    if (!type_system)
>>          return false;
>> 
>>      // fetch children by offset to compensate for potential lack of debug info
>> -    auto int64_ty = ast_ctx->GetIntTypeFromBitSize(64, true);
>> -    auto int32_ty = ast_ctx->GetIntTypeFromBitSize(32, true);
>> +    auto int64_ty = type_system->GetIntTypeFromBitSize(64, true);
>> +    auto int32_ty = type_system->GetIntTypeFromBitSize(32, true);
>> 
>>      auto value_sp(valobj.GetSyntheticChildAtOffset(0, int64_ty, true));
>>      auto timescale_sp(valobj.GetSyntheticChildAtOffset(8, int32_ty, true));
>> 
>> Modified: lldb/trunk/source/DataFormatters/VectorType.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/VectorType.cpp?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/VectorType.cpp?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/source/DataFormatters/VectorType.cpp (original)
>> +++ lldb/trunk/source/DataFormatters/VectorType.cpp Tue Sep  8 17:09:19 2015
>> @@ -11,8 +11,8 @@
>> 
>>  #include "lldb/Core/ValueObject.h"
>>  #include "lldb/DataFormatters/FormattersHelpers.h"
>> -#include "lldb/Symbol/ClangASTContext.h"
>>  #include "lldb/Symbol/CompilerType.h"
>> +#include "lldb/Symbol/TypeSystem.h"
>> 
>>  #include "lldb/Utility/LLDBAssert.h"
>> 
>> @@ -22,85 +22,85 @@ using namespace lldb_private::formatters
>> 
>>  static CompilerType
>>  GetCompilerTypeForFormat (lldb::Format format,
>> -                       CompilerType element_type,
>> -                       ClangASTContext *ast_ctx)
>> +                          CompilerType element_type,
>> +                          TypeSystem *type_system)
>>  {
>> -    lldbassert(ast_ctx && "ast_ctx needs to be not NULL");
>> +    lldbassert(type_system && "type_system needs to be not NULL");
>> 
>>      switch (format)
>>      {
>>          case lldb::eFormatAddressInfo:
>>          case lldb::eFormatPointer:
>> -            return ast_ctx->GetPointerSizedIntType(false);
>> +            return type_system->GetIntTypeFromBitSize(8*type_system->GetPointerByteSize(), false);
>> 
>>          case lldb::eFormatBoolean:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeBool);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeBool);
>> 
>>          case lldb::eFormatBytes:
>>          case lldb::eFormatBytesWithASCII:
>>          case lldb::eFormatChar:
>>          case lldb::eFormatCharArray:
>>          case lldb::eFormatCharPrintable:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeChar);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeChar);
>> 
>>          case lldb::eFormatComplex /* lldb::eFormatComplexFloat */:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeFloatComplex);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloatComplex);
>> 
>>          case lldb::eFormatCString:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeChar).GetPointerType();
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeChar).GetPointerType();
>> 
>>          case lldb::eFormatFloat:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeFloat);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloat);
>> 
>>          case lldb::eFormatHex:
>>          case lldb::eFormatHexUppercase:
>>          case lldb::eFormatOctal:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeInt);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeInt);
>> 
>>          case lldb::eFormatHexFloat:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeFloat);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloat);
>> 
>>          case lldb::eFormatUnicode16:
>>          case lldb::eFormatUnicode32:
>> 
>>          case lldb::eFormatUnsigned:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedInt);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeUnsignedInt);
>> 
>>          case lldb::eFormatVectorOfChar:
>> -            return ast_ctx->GetBasicType(lldb::eBasicTypeChar);
>> +            return type_system->GetBasicTypeFromAST(lldb::eBasicTypeChar);
>> 
>>          case lldb::eFormatVectorOfFloat32:
>> -            return ast_ctx->GetFloatTypeFromBitSize(32);
>> +            return type_system->GetFloatTypeFromBitSize(32);
>> 
>>          case lldb::eFormatVectorOfFloat64:
>> -            return ast_ctx->GetFloatTypeFromBitSize(64);
>> +            return type_system->GetFloatTypeFromBitSize(64);
>> 
>>          case lldb::eFormatVectorOfSInt16:
>> -            return ast_ctx->GetIntTypeFromBitSize(16, true);
>> +            return type_system->GetIntTypeFromBitSize(16, true);
>> 
>>          case lldb::eFormatVectorOfSInt32:
>> -            return ast_ctx->GetIntTypeFromBitSize(32, true);
>> +            return type_system->GetIntTypeFromBitSize(32, true);
>> 
>>          case lldb::eFormatVectorOfSInt64:
>> -            return ast_ctx->GetIntTypeFromBitSize(64, true);
>> +            return type_system->GetIntTypeFromBitSize(64, true);
>> 
>>          case lldb::eFormatVectorOfSInt8:
>> -            return ast_ctx->GetIntTypeFromBitSize(8, true);
>> +            return type_system->GetIntTypeFromBitSize(8, true);
>> 
>>          case lldb::eFormatVectorOfUInt128:
>> -            return ast_ctx->GetIntTypeFromBitSize(128, false);
>> +            return type_system->GetIntTypeFromBitSize(128, false);
>> 
>>          case lldb::eFormatVectorOfUInt16:
>> -            return ast_ctx->GetIntTypeFromBitSize(16, false);
>> +            return type_system->GetIntTypeFromBitSize(16, false);
>> 
>>          case lldb::eFormatVectorOfUInt32:
>> -            return ast_ctx->GetIntTypeFromBitSize(32, false);
>> +            return type_system->GetIntTypeFromBitSize(32, false);
>> 
>>          case lldb::eFormatVectorOfUInt64:
>> -            return ast_ctx->GetIntTypeFromBitSize(64, false);
>> +            return type_system->GetIntTypeFromBitSize(64, false);
>> 
>>          case lldb::eFormatVectorOfUInt8:
>> -            return ast_ctx->GetIntTypeFromBitSize(8, false);
>> +            return type_system->GetIntTypeFromBitSize(8, false);
>> 
>>          case lldb::eFormatDefault:
>>              return element_type;
>> @@ -113,7 +113,7 @@ GetCompilerTypeForFormat (lldb::Format f
>>          case lldb::eFormatOSType:
>>          case lldb::eFormatVoid:
>>          default:
>> -            return ast_ctx->GetIntTypeFromBitSize(8, false);
>> +            return type_system->GetIntTypeFromBitSize(8, false);
>>      }
>>  }
>> 
>> @@ -232,7 +232,10 @@ namespace lldb_private {
>>                  CompilerType parent_type(m_backend.GetCompilerType());
>>                  CompilerType element_type;
>>                  parent_type.IsVectorType(&element_type, nullptr);
>> -                m_child_type = ::GetCompilerTypeForFormat(m_parent_format, element_type, llvm::dyn_cast_or_null<ClangASTContext>(parent_type.GetTypeSystem()));
>> +                TargetSP target_sp(m_backend.GetTargetSP());
>> +                m_child_type = ::GetCompilerTypeForFormat(m_parent_format,
>> +                                                          element_type,
>> +                                                          target_sp ? target_sp->GetTypeSystemForLanguage(lldb::eLanguageTypeC) : nullptr);
>>                  m_num_children = ::CalculateNumChildren(parent_type,
>>                                                          m_child_type);
>>                  m_item_format = GetItemFormatForFormat(m_parent_format,
>> 
>> Modified: lldb/trunk/source/Target/Target.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=247082&r1=247081&r2=247082&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=247082&r1=247081&r2=247082&view=diff>
>> ==============================================================================
>> --- lldb/trunk/source/Target/Target.cpp (original)
>> +++ lldb/trunk/source/Target/Target.cpp Tue Sep  8 17:09:19 2015
>> @@ -1906,6 +1906,27 @@ Target::GetScratchClangASTContext(bool c
>>      return m_scratch_ast_context_ap.get();
>>  }
>> 
>> +TypeSystem*
>> +Target::GetTypeSystemForLanguage (lldb::LanguageType language)
>> +{
>> +    switch (language)
>> +    {
>> +        case lldb::eLanguageTypeC:
>> +        case lldb::eLanguageTypeC11:
>> +        case lldb::eLanguageTypeC89:
>> +        case lldb::eLanguageTypeC99:
>> +        case lldb::eLanguageTypeC_plus_plus:
>> +        case lldb::eLanguageTypeC_plus_plus_03:
>> +        case lldb::eLanguageTypeC_plus_plus_11:
>> +        case lldb::eLanguageTypeC_plus_plus_14:
>> +        case lldb::eLanguageTypeObjC:
>> +        case lldb::eLanguageTypeObjC_plus_plus:
>> +            return GetScratchClangASTContext(true);
>> +        default:
>> +            return nullptr;
>> +    }
>> +}
>> +
>>  ClangASTImporter *
>>  Target::GetClangASTImporter()
>>  {
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org <mailto:lldb-commits at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150910/74418b82/attachment-0001.html>


More information about the lldb-commits mailing list