Re: [Lldb-commits] [lldb] fe8e25a - [lldb][NFC] Create type-safe function for creating a CompilerType from a QualType
Somehow your mail is missing examples after the ‘… like:’ but the method is
named similar to the GetType* methods directly below it. Don’t really have an
opinion about whether this should be GetType* or GetCompilerType* (though I
wouldn’t add the ‘As’ to it though as that’s usually used for casting
utilities). I can rename the whole pack of function to GetCompilerType(…), that
sounds good to me.
> On Jan 2, 2020, at 11:05 PM, Shafik Yaghmour wrote:
>
> Instead of GetType(...) I think GetCompilerType(…) is better or maybe even
> GetAsCompilerType(…). It adds a bit more context, we have so many types. It
> also feels a bit more consistent with methods like:
>
>
>
>> On Jan 2, 2020, at 2:55 AM, Raphael Isemann via lldb-commits
>> wrote:
>>
>>
>> Author: Raphael Isemann
>> Date: 2020-01-02T11:54:45+01:00
>> New Revision: fe8e25a48a2a0f8f508499ba950181dba3d600b0
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0.diff
>>
>> LOG: [lldb][NFC] Create type-safe function for creating a CompilerType from
>> a QualType
>>
>> LLDB frequently converts QualType to CompilerType. This is currently done
>> like this:
>> result = CompilerType(this, qual_type_var.getAsOpaquePtr())
>> There are a few shortcomings in this current approach:
>> 1. CompilerType's constructor takes a void* pointer so it isn't type safe.
>> 2. We can't add any sanity checks to the CompilerType constructor (e.g. that
>> the type
>>actually belongs to the passed ClangASTContext) without expanding the
>> TypeSystem API.
>> 3. The logic for converting QualType->CompilerType is spread out over all of
>> LLDB so
>>changing it is difficult (e.g., what if we want to just pass the type ptr
>> and not the
>>1type_ptr | qual_flags1 to CompilerType).
>>
>> This patch adds a `ClangASTContext::GetType` function similar to the other
>> GetTypeForDecl
>> functions that does this conversion in a type safe way.
>>
>> It also adds a sanity check for Tag-based types that the type actually
>> belongs to the
>> current ClangASTContext (Types don't seem to know their ASTContext, so we
>> have to
>> workaround by looking at the decl for the underlying TagDecl. This doesn't
>> cover all types
>> we construct but it's better than no sanity check).
>>
>> Added:
>>
>>
>> Modified:
>> lldb/include/lldb/Symbol/ClangASTContext.h
>> lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> lldb/source/Plugins/Language/ObjC/NSArray.cpp
>>
>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
>> lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>> lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
>> lldb/source/Symbol/ClangASTContext.cpp
>>
>> Removed:
>>
>>
>>
>>
>> diff --git a/lldb/include/lldb/Symbol/ClangASTContext.h
>> b/lldb/include/lldb/Symbol/ClangASTContext.h
>> index e9a1d536ca8e..53ecd1bb78fc 100644
>> --- a/lldb/include/lldb/Symbol/ClangASTContext.h
>> +++ b/lldb/include/lldb/Symbol/ClangASTContext.h
>> @@ -164,6 +164,22 @@ class ClangASTContext : public TypeSystem {
>> static bool AreTypesSame(CompilerType type1, CompilerType type2,
>> bool ignore_qualifiers = false);
>>
>> + /// Creates a CompilerType form the given QualType with the current
>> + /// ClangASTContext instance as the CompilerType's typesystem.
>> + /// \param qt The QualType for a type that belongs to the ASTContext of
>> this
>> + /// ClangASTContext.
>> + /// \return The CompilerType representing the given QualType. If the
>> + /// QualType's type pointer is a nullptr then the function
>> returns an
>> + /// invalid CompilerType.
>> + CompilerType GetType(clang::QualType qt) {
>> +if (qt.getTypePtrOrNull() == nullptr)
>> + return CompilerType();
>> +// Check that the type actually belongs to this ClangASTContext.
>> +assert(qt->getAsTagDecl() == nullptr ||
>> + >getAsTagDecl()->getASTContext() == ());
>> +return CompilerType(this, qt.getAsOpaquePtr());
>> + }
>> +
>> CompilerType GetTypeForDecl(clang::NamedDecl *decl);
>>
>> CompilerType GetTypeForDecl(clang::TagDecl *decl);
>>
>> diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> index ff86f9f818b2..b0043f9c0f64 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> @@ -1861,7 +1861,7 @@ CompilerType ClangASTSource::GuardedCopyType(const
>> CompilerType _type) {
>>// seems to be generating bad types on occasion.
>>return CompilerType();
>>
>> - return CompilerType(m_clang_ast_context,
>>
Re: [Lldb-commits] [lldb] fe8e25a - [lldb][NFC] Create type-safe function for creating a CompilerType from a QualType
Instead of GetType(...) I think GetCompilerType(…) is better or maybe even
GetAsCompilerType(…). It adds a bit more context, we have so many types. It
also feels a bit more consistent with methods like:
> On Jan 2, 2020, at 2:55 AM, Raphael Isemann via lldb-commits
> wrote:
>
>
> Author: Raphael Isemann
> Date: 2020-01-02T11:54:45+01:00
> New Revision: fe8e25a48a2a0f8f508499ba950181dba3d600b0
>
> URL:
> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0
> DIFF:
> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0.diff
>
> LOG: [lldb][NFC] Create type-safe function for creating a CompilerType from a
> QualType
>
> LLDB frequently converts QualType to CompilerType. This is currently done
> like this:
>result = CompilerType(this, qual_type_var.getAsOpaquePtr())
> There are a few shortcomings in this current approach:
> 1. CompilerType's constructor takes a void* pointer so it isn't type safe.
> 2. We can't add any sanity checks to the CompilerType constructor (e.g. that
> the type
> actually belongs to the passed ClangASTContext) without expanding the
> TypeSystem API.
> 3. The logic for converting QualType->CompilerType is spread out over all of
> LLDB so
> changing it is difficult (e.g., what if we want to just pass the type ptr
> and not the
> 1type_ptr | qual_flags1 to CompilerType).
>
> This patch adds a `ClangASTContext::GetType` function similar to the other
> GetTypeForDecl
> functions that does this conversion in a type safe way.
>
> It also adds a sanity check for Tag-based types that the type actually
> belongs to the
> current ClangASTContext (Types don't seem to know their ASTContext, so we
> have to
> workaround by looking at the decl for the underlying TagDecl. This doesn't
> cover all types
> we construct but it's better than no sanity check).
>
> Added:
>
>
> Modified:
>lldb/include/lldb/Symbol/ClangASTContext.h
>lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>lldb/source/Plugins/Language/ObjC/NSArray.cpp
>
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
>lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
>lldb/source/Symbol/ClangASTContext.cpp
>
> Removed:
>
>
>
>
> diff --git a/lldb/include/lldb/Symbol/ClangASTContext.h
> b/lldb/include/lldb/Symbol/ClangASTContext.h
> index e9a1d536ca8e..53ecd1bb78fc 100644
> --- a/lldb/include/lldb/Symbol/ClangASTContext.h
> +++ b/lldb/include/lldb/Symbol/ClangASTContext.h
> @@ -164,6 +164,22 @@ class ClangASTContext : public TypeSystem {
> static bool AreTypesSame(CompilerType type1, CompilerType type2,
>bool ignore_qualifiers = false);
>
> + /// Creates a CompilerType form the given QualType with the current
> + /// ClangASTContext instance as the CompilerType's typesystem.
> + /// \param qt The QualType for a type that belongs to the ASTContext of
> this
> + /// ClangASTContext.
> + /// \return The CompilerType representing the given QualType. If the
> + /// QualType's type pointer is a nullptr then the function returns
> an
> + /// invalid CompilerType.
> + CompilerType GetType(clang::QualType qt) {
> +if (qt.getTypePtrOrNull() == nullptr)
> + return CompilerType();
> +// Check that the type actually belongs to this ClangASTContext.
> +assert(qt->getAsTagDecl() == nullptr ||
> + >getAsTagDecl()->getASTContext() == ());
> +return CompilerType(this, qt.getAsOpaquePtr());
> + }
> +
> CompilerType GetTypeForDecl(clang::NamedDecl *decl);
>
> CompilerType GetTypeForDecl(clang::TagDecl *decl);
>
> diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
> b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
> index ff86f9f818b2..b0043f9c0f64 100644
> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
> @@ -1861,7 +1861,7 @@ CompilerType ClangASTSource::GuardedCopyType(const
> CompilerType _type) {
> // seems to be generating bad types on occasion.
> return CompilerType();
>
> - return CompilerType(m_clang_ast_context,
> copied_qual_type.getAsOpaquePtr());
> + return m_clang_ast_context->GetType(copied_qual_type);
> }
>
> clang::NamedDecl *NameSearchContext::AddVarDecl(const CompilerType ) {
> @@ -1988,9 +1988,8 @@ clang::NamedDecl
> *NameSearchContext::AddGenericFunDecl() {
> ArrayRef(), // argument types
> proto_info));
>
> - return AddFunDecl(CompilerType(m_ast_source.m_clang_ast_context,
> - generic_function_type.getAsOpaquePtr()),
> -true);
> + return AddFunDecl(
> +
[Lldb-commits] [lldb] fe8e25a - [lldb][NFC] Create type-safe function for creating a CompilerType from a QualType
Author: Raphael Isemann
Date: 2020-01-02T11:54:45+01:00
New Revision: fe8e25a48a2a0f8f508499ba950181dba3d600b0
URL:
https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0
DIFF:
https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0.diff
LOG: [lldb][NFC] Create type-safe function for creating a CompilerType from a
QualType
LLDB frequently converts QualType to CompilerType. This is currently done like
this:
result = CompilerType(this, qual_type_var.getAsOpaquePtr())
There are a few shortcomings in this current approach:
1. CompilerType's constructor takes a void* pointer so it isn't type safe.
2. We can't add any sanity checks to the CompilerType constructor (e.g. that
the type
actually belongs to the passed ClangASTContext) without expanding the
TypeSystem API.
3. The logic for converting QualType->CompilerType is spread out over all of
LLDB so
changing it is difficult (e.g., what if we want to just pass the type ptr
and not the
1type_ptr | qual_flags1 to CompilerType).
This patch adds a `ClangASTContext::GetType` function similar to the other
GetTypeForDecl
functions that does this conversion in a type safe way.
It also adds a sanity check for Tag-based types that the type actually belongs
to the
current ClangASTContext (Types don't seem to know their ASTContext, so we have
to
workaround by looking at the decl for the underlying TagDecl. This doesn't
cover all types
we construct but it's better than no sanity check).
Added:
Modified:
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/Language/ObjC/NSArray.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Symbol/ClangASTContext.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/ClangASTContext.h
b/lldb/include/lldb/Symbol/ClangASTContext.h
index e9a1d536ca8e..53ecd1bb78fc 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -164,6 +164,22 @@ class ClangASTContext : public TypeSystem {
static bool AreTypesSame(CompilerType type1, CompilerType type2,
bool ignore_qualifiers = false);
+ /// Creates a CompilerType form the given QualType with the current
+ /// ClangASTContext instance as the CompilerType's typesystem.
+ /// \param qt The QualType for a type that belongs to the ASTContext of this
+ /// ClangASTContext.
+ /// \return The CompilerType representing the given QualType. If the
+ /// QualType's type pointer is a nullptr then the function returns an
+ /// invalid CompilerType.
+ CompilerType GetType(clang::QualType qt) {
+if (qt.getTypePtrOrNull() == nullptr)
+ return CompilerType();
+// Check that the type actually belongs to this ClangASTContext.
+assert(qt->getAsTagDecl() == nullptr ||
+ >getAsTagDecl()->getASTContext() == ());
+return CompilerType(this, qt.getAsOpaquePtr());
+ }
+
CompilerType GetTypeForDecl(clang::NamedDecl *decl);
CompilerType GetTypeForDecl(clang::TagDecl *decl);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index ff86f9f818b2..b0043f9c0f64 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -1861,7 +1861,7 @@ CompilerType ClangASTSource::GuardedCopyType(const
CompilerType _type) {
// seems to be generating bad types on occasion.
return CompilerType();
- return CompilerType(m_clang_ast_context, copied_qual_type.getAsOpaquePtr());
+ return m_clang_ast_context->GetType(copied_qual_type);
}
clang::NamedDecl *NameSearchContext::AddVarDecl(const CompilerType ) {
@@ -1988,9 +1988,8 @@ clang::NamedDecl *NameSearchContext::AddGenericFunDecl() {
ArrayRef(), // argument types
proto_info));
- return AddFunDecl(CompilerType(m_ast_source.m_clang_ast_context,
- generic_function_type.getAsOpaquePtr()),
-true);
+ return AddFunDecl(
+ m_ast_source.m_clang_ast_context->GetType(generic_function_type), true);
}
clang::NamedDecl *
diff --git a/lldb/source/Plugins/Language/ObjC/NSArray.cpp
b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
index 64461fc2bc0f..0ac7fb6d2330 100644
--- a/lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -612,9 +612,8 @@
lldb_private::formatters::GenericNSArrayISyntheticFrontEnd::
auto *clang_ast_context =
