Overall a good start. I am confused as to why there were a lot of:
clang_type.method(...)
changed to
type_system->method(clang_type, ...)
Seems like we should keep the old API as the ClangASTType contains a
"TypeSystem*" and it would make the diffs a lot less noisy in SymbolFileDWARF.
These are all of the "Why is this no longer a method on ..." inline code
notations.
It also seems like anything that is actually creating a ClangASTTypeSystem
specific type should have the creation functions only in ClangASTTypeSystem and
not make all of them pure virtual in the TypeSystem definition. Then each
language will have its own TypeSystem subclass. See inlined notes that say
"move to ClangASTTypeSystem.h?". If we do this, then each module needs to be
able to get one of the TypeSystem specific subclasses from the module:
ClangASTTypeSystem *
Module::GetTypeSystemClang ();
ClangASTTypeGo *
Module::GetTypeSystemGo ();
ClangASTTypeSwift *
Module::GetTypeSystemSwift ();
Then the DWARF parser would grab the right TypeSystem subclass from the module
in SymbolFileDWARF and use that to create types. Each TypeSystem subclass can
have its own way to create native types. All of which must return ClangASTType
(this really should be renamed CompilerType) types.
So the changes I would like to see is:
1 - Make ClangASTContext inherit from TypeSystem
2 - Try and isolate all of the language specific type creation routines into
ClangASTContext (since it now inherits from TypeSystem) and let it create
native clang types. This way not all languages have to make hundreds of pure
virtual functions that they will never support. If any are generic enough, we
can include them, but it would be cleaner to allow each langue to have its own
language specific create/addField/addIVar, etc routines
3 - Add get_as<T>() to the TypeSystem and allow people to dyn_cast their
TypeSystem into more specific subclasses where needed and where the pure
virtual stuff falls down.
REPOSITORY
rL LLVM
================
Comment at: include/lldb/Symbol/ClangASTContext.h:39
@@ -37,3 +38,3 @@
class ClangASTContext
{
----------------
Shouldn't this inherit from TypeSystem?
================
Comment at: include/lldb/Symbol/ClangASTContext.h:58-59
@@ -56,1 +57,4 @@
+
+ ClangASTTypeSystem*
+ getTypeSystem();
----------------
If ClangASTContext inherits from TypeSystem, this isn't needed.
================
Comment at: include/lldb/Symbol/ClangASTContext.h:509
@@ -490,2 +508,3 @@
std::unique_ptr<clang::Builtin::Context> m_builtins_ap;
+ std::unique_ptr<ClangASTTypeSystem> m_type_system_ap;
CompleteTagDeclCallback m_callback_tag_decl;
----------------
Shouldn't ClangASTContext inherit from TypeSystem? If so, this isn't needed.
================
Comment at: include/lldb/Symbol/ClangASTType.h:20
@@ -19,1 +19,3 @@
+class TypeSystem;
+
----------------
Put this in lldb-forward.h and remove any other forward references to
TypeSystem.
================
Comment at: include/lldb/Symbol/ClangASTType.h:38
@@ -35,8 +37,3 @@
//----------------------------------------------------------------------
- ClangASTType (clang::ASTContext *ast_context, lldb::clang_type_t type) :
- m_type (type),
- m_ast (ast_context)
- {
- }
-
+ ClangASTType (clang::ASTContext *ast_context, lldb::clang_type_t type);
ClangASTType (clang::ASTContext *ast_context, clang::QualType qual_type);
----------------
This should be "TypeSystem* type_system, void *type".
================
Comment at: include/lldb/Symbol/ClangASTType.h:233-235
@@ -235,2 +232,5 @@
+ clang::ASTContext *
+ GetASTContext() const;
+
ConstString
----------------
We shouldn't need this we should just be able to use the GetTypeSystem() above.
We might need to implement llvm style dyn_cast<> operators on TypeSystem.
================
Comment at: include/lldb/Symbol/ClangASTType.h:264
@@ -263,8 +263,3 @@
void
- SetClangType (clang::ASTContext *ast, lldb::clang_type_t type)
- {
- m_ast = ast;
- m_type = type;
- }
-
+ SetClangType (clang::ASTContext *ast, lldb::clang_type_t type);
void
----------------
This should be "TypeSystem* type_system, void *type".
================
Comment at: include/lldb/Symbol/TypeSystem.h:189-211
@@ +188,25 @@
+
+ virtual ClangASTType
+ AddConstModifier (void * type) const = 0;
+
+ virtual ClangASTType
+ AddRestrictModifier (void * type) const = 0;
+
+ virtual ClangASTType
+ AddVolatileModifier (void * type) const = 0;
+
+ // Using the current type, create a new typedef to that type using
"typedef_name"
+ // as the name and "decl_ctx" as the decl context.
+ virtual ClangASTType
+ CreateTypedefType (void * type, const char *typedef_name,
+ clang::DeclContext *decl_ctx) const = 0;
+
+ virtual ClangASTType
+ GetArrayElementType (void * type, uint64_t *stride = nullptr) const = 0;
+
+ virtual ClangASTType
+ GetCanonicalType (void * type) const = 0;
+
+ virtual ClangASTType
+ GetFullyUnqualifiedType (void * type) const = 0;
+
----------------
move to ClangASTTypeSystem.h?
We might not need these in the base class since they are used to create
ClangASTTypeSystem based types. Each language should have their own creation
types that not every language should have to implement.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1715
@@ -1713,8 +1714,3 @@
{
- return m_class_opaque_type.AddObjCClassProperty (m_property_name,
-
m_property_opaque_type,
- m_ivar_decl,
-
m_property_setter_name,
-
m_property_getter_name,
- m_property_attributes,
- m_metadata_ap.get());
+ return
static_cast<ClangASTTypeSystem*>(m_class_opaque_type.GetTypeSystem())->
+ AddObjCClassProperty (m_class_opaque_type.GetOpaqueQualType(),
----------------
Do we want some sort of get_as<ClangASTTypeSystem>() to make sure we don't try
this on another type system type?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1817
@@ -1813,2 +1816,3 @@
ModuleSP module = GetObjectFile()->GetModule();
+ ClangASTTypeSystem* types =
static_cast<ClangASTTypeSystem*>(class_clang_type.GetTypeSystem());
----------------
Do we want some sort of get_as<ClangASTTypeSystem>() to make sure we don't try
this on another type system type?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1817
@@ -1813,2 +1816,3 @@
ModuleSP module = GetObjectFile()->GetModule();
+ ClangASTTypeSystem* types =
static_cast<ClangASTTypeSystem*>(class_clang_type.GetTypeSystem());
----------------
clayborg wrote:
> Do we want some sort of get_as<ClangASTTypeSystem>() to make sure we don't
> try this on another type system type?
We should make a common name for any TypeSystem variable like "type_system" and
use these common names everywhere. "types" seems like the wrong variable name
here for clarity sake.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2006
@@ -2001,4 +2005,3 @@
accessibility = eAccessPublic;
- class_clang_type.AddVariableToRecordType (name,
-
var_type->GetClangLayoutType(),
-
accessibility);
+ types->AddVariableToRecordType
(class_clang_type.GetOpaqueQualType(),
+ name,
----------------
Why is this no longer a method on "class_clang_type"?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2124
@@ -2118,3 +2123,3 @@
{
- clang::FieldDecl
*unnamed_bitfield_decl = class_clang_type.AddFieldToRecordType (NULL,
+ clang::FieldDecl
*unnamed_bitfield_decl = types->AddFieldToRecordType
(class_clang_type.GetOpaqueQualType(), NULL,
GetClangASTContext().GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
word_width),
----------------
Why is this no longer a method on "class_clang_type"?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2174
@@ -2168,3 +2173,3 @@
- field_decl =
class_clang_type.AddFieldToRecordType (name,
+ field_decl = types->AddFieldToRecordType
(class_clang_type.GetOpaqueQualType(), name,
member_clang_type,
----------------
Why is this no longer a method on "class_clang_type"?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2322
@@ -2316,3 +2321,3 @@
{
-
class_clang_type.SetObjCSuperClass(base_class_clang_type);
+
types->SetObjCSuperClass(class_clang_type.GetOpaqueQualType(),
base_class_clang_type);
}
----------------
Why is this no longer a method on "class_clang_type"?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2326
@@ -2320,4 +2325,3 @@
{
- base_classes.push_back
(base_class_clang_type.CreateBaseClassSpecifier (accessibility,
-
is_virtual,
-
is_base_of_class));
+ base_classes.push_back
(types->CreateBaseClassSpecifier (base_class_clang_type.GetOpaqueQualType(),
+
accessibility,
----------------
Why is this no longer a method on base_class_clang_type?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2492
@@ -2483,3 +2491,3 @@
// clang::ExternalASTSource queries for this type.
- clang_type.SetHasExternalStorage (false);
+ types->SetHasExternalStorage (clang_type.GetOpaqueQualType(), false);
----------------
Why is this no longer a method on clang_type?
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2528
@@ -2519,3 +2527,3 @@
// the class is created.
- clang_type.StartTagDeclarationDefinition ();
+ types->StartTagDeclarationDefinition
(clang_type.GetOpaqueQualType());
}
----------------
Why no longer a method on the object like before?
http://reviews.llvm.org/D8712
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
