[Lldb-commits] [PATCH] Introduce a TypeSystem interface to support adding non-clang languages.
Greg Clayton
clayborg at gmail.com
Mon Mar 30 16:05:45 PDT 2015
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/
More information about the lldb-commits
mailing list