Should we go back to my original patch then? It left the parser accessible from TypeSystem, but made it a DebugInfoParser instead of a DWARFParser.<br><br>The whole idea is to remove knowledge of a specific debug info format from the generic TypeSystem, so it still seems wrong to add DWARF specific stuff to TypeSystem.<br><br>One option is to add back the SymbolFormat enum and make a method in TypeSystem like<br><br>DebugInfoASTParser *<br>GetASTParserForDebugFormat(SymbolFormat format)<br><br>This way it's not DWARF specific.<br><br>Another option is to add a method for pdb and a method for DWARF. I think the former is more flexible though.<br><div class="gmail_quote"><div dir="ltr">On Tue, Mar 22, 2016 at 4:22 PM Greg Clayton <<a href="mailto:clayborg@gmail.com">clayborg@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">clayborg requested changes to this revision.<br>
clayborg added a comment.<br>
This revision now requires changes to proceed.<br>
<br>
After our last conversation I must have missed that both Go and Java need to be able to provide custom DWARF parsing. Any language, and any TypeSystem, should be able to provide a DWARFASTParser. They don't have to, but they can. So we will need to revert ALL the changes that remove the GetDWARFParser from the TypeSystem and restore them.<br>
<br>
This patch them becomes a method to factor the clang::ExternalASTSource stuff only so that you will be able to use it in the SymbolFilePDB. Sorry for the confusion..<br>
<br>
<br>
================<br>
Comment at: include/lldb/Symbol/ClangASTContext.h:37-39<br>
@@ -37,2 +36,5 @@<br>
#include "lldb/Symbol/TypeSystem.h"<br>
+#include "lldb/lldb-enumerations.h"<br>
+<br>
+class DWARFASTParserClang;<br>
<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/ClangASTContext.h:69-71<br>
@@ -66,2 +68,5 @@<br>
<br>
+ DWARFASTParserClang *<br>
+ GetDWARFParser();<br>
+<br>
//------------------------------------------------------------------<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/ClangASTContext.h:521-526<br>
@@ -520,8 +525,2 @@<br>
//------------------------------------------------------------------<br>
- // TypeSystem methods<br>
- //------------------------------------------------------------------<br>
- DWARFASTParser *<br>
- GetDWARFParser () override;<br>
-<br>
- //------------------------------------------------------------------<br>
// ClangASTContext callbacks for external source lookups.<br>
----------------<br>
We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/ClangASTContext.h:1228<br>
@@ -1227,3 +1227,3 @@<br>
std::unique_ptr<clang::Builtin::Context> m_builtins_ap;<br>
- std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;<br>
+ std::unique_ptr<DWARFASTParserClang> m_dwarf_ast_parser_ap;<br>
std::unique_ptr<ClangASTSource> m_scratch_ast_source_ap;<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/GoASTContext.h:63-64<br>
@@ -62,4 +62,2 @@<br>
-<br>
- DWARFASTParser *GetDWARFParser() override;<br>
<br>
void<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/GoASTContext.h:390<br>
@@ -389,3 +387,2 @@<br>
std::unique_ptr<TypeMap> m_types;<br>
- std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;<br>
<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/JavaASTContext.h:61-63<br>
@@ -60,5 +60,2 @@<br>
<br>
- DWARFASTParser *<br>
- GetDWARFParser() override;<br>
-<br>
uint32_t<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/JavaASTContext.h:371<br>
@@ -370,3 +367,2 @@<br>
uint32_t m_pointer_byte_size;<br>
- std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;<br>
JavaTypeMap m_array_type_map;<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/TypeSystem.h:31<br>
@@ -30,3 +30,1 @@<br>
<br>
-class DWARFDIE;<br>
-class DWARFASTParser;<br>
----------------<br>
This line can still go as it isn't needed.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/TypeSystem.h:32<br>
@@ -31,3 @@<br>
-class DWARFDIE;<br>
-class DWARFASTParser;<br>
-<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: include/lldb/Symbol/TypeSystem.h:102-107<br>
@@ -101,8 +98,2 @@<br>
<br>
- virtual DWARFASTParser *<br>
- GetDWARFParser ()<br>
- {<br>
- return nullptr;<br>
- }<br>
-<br>
virtual SymbolFile *<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:12-30<br>
@@ -11,17 +11,21 @@<br>
<br>
+#include "DWARFASTParser.h"<br>
#include "DWARFCompileUnit.h"<br>
+#include "DWARFDIECollection.h"<br>
#include "DWARFDebugAbbrev.h"<br>
#include "DWARFDebugAranges.h"<br>
#include "DWARFDebugInfo.h"<br>
#include "DWARFDebugInfoEntry.h"<br>
#include "DWARFDebugRanges.h"<br>
#include "DWARFDeclContext.h"<br>
-#include "DWARFDIECollection.h"<br>
#include "DWARFFormValue.h"<br>
#include "SymbolFileDWARF.h"<br>
<br>
#include "lldb/Core/Module.h"<br>
+#include "lldb/Symbol/ClangASTContext.h"<br>
#include "lldb/Symbol/ObjectFile.h"<br>
#include "lldb/Symbol/Type.h"<br>
#include "lldb/Symbol/TypeSystem.h"<br>
<br>
+#include "Plugins\SymbolFile\DWARF\DWARFASTParserClang.h"<br>
+<br>
----------------<br>
Revert all changes in this file. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:408-415<br>
@@ -403,11 +407,10 @@<br>
<br>
DWARFASTParser *<br>
DWARFDIE::GetDWARFParser () const<br>
{<br>
- lldb_private::TypeSystem *type_system = GetTypeSystem ();<br>
- if (type_system)<br>
- return type_system->GetDWARFParser();<br>
- else<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(GetTypeSystem());<br>
+ if (!clang_type_system)<br>
return nullptr;<br>
+ return clang_type_system->GetDWARFParser();<br>
}<br>
<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:57<br>
@@ -56,2 +56,3 @@<br>
#include "DWARFASTParser.h"<br>
+#include "DWARFASTParserClang.h"<br>
#include "DWARFCompileUnit.h"<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:997-1000<br>
@@ -995,6 +996,6 @@<br>
TypeSystem *type_system = GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());<br>
-<br>
- if (type_system)<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);<br>
+ if (clang_type_system)<br>
{<br>
- DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();<br>
+ DWARFASTParser *dwarf_ast = clang_type_system->GetDWARFParser();<br>
if (dwarf_ast)<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1465-1469<br>
@@ -1463,3 +1464,7 @@<br>
TypeSystem *type_system = decl_ctx.GetTypeSystem();<br>
- DWARFASTParser *ast_parser = type_system->GetDWARFParser();<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);<br>
+ if (!clang_type_system)<br>
+ return;<br>
+<br>
+ DWARFASTParser *ast_parser = clang_type_system->GetDWARFParser();<br>
std::vector<DWARFDIE> decl_ctx_die_list = ast_parser->GetDIEForDeclContext(decl_ctx);<br>
----------------<br>
Revert. We will need to restore this since Go and Java TypeSystem subclasses need to provide DWARF parsers that translate the DWARF into types for those languages in their type systems. This is my fault.. So just leave this as is.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1617-1625<br>
@@ -1611,9 +1616,11 @@<br>
TypeSystem *type_system = compiler_type.GetTypeSystem();<br>
- if (type_system)<br>
- {<br>
- DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();<br>
- if (dwarf_ast)<br>
- return dwarf_ast->CanCompleteType(compiler_type);<br>
- }<br>
- return false;<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);<br>
+ if (!clang_type_system)<br>
+ return false;<br>
+<br>
+ DWARFASTParserClang *ast_parser = clang_type_system->GetDWARFParser();<br>
+ if (!ast_parser)<br>
+ return false;<br>
+<br>
+ return ast_parser->CanCompleteType(compiler_type);<br>
}<br>
----------------<br>
Revert unless you need to route this to your new Clang external source changes you made.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1635-1638<br>
@@ -1627,5 +1634,6 @@<br>
TypeSystem *type_system = compiler_type.GetTypeSystem();<br>
- if (type_system)<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);<br>
+ if (clang_type_system)<br>
{<br>
- DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();<br>
+ DWARFASTParserClang *dwarf_ast = clang_type_system->GetDWARFParser();<br>
if (dwarf_ast && dwarf_ast->CanCompleteType(compiler_type))<br>
----------------<br>
revert.<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3828-3846<br>
@@ -3819,6 +3827,21 @@<br>
<br>
- if (die)<br>
- {<br>
- TypeSystem *type_system = GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());<br>
+ if (!die)<br>
+ return TypeSP();<br>
<br>
- if (type_system)<br>
+ TypeSystem *type_system = GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());<br>
+ ClangASTContext *clang_type_system = llvm::dyn_cast_or_null<ClangASTContext>(type_system);<br>
+ if (!clang_type_system)<br>
+ return TypeSP();<br>
+<br>
+ DWARFASTParser *dwarf_ast = clang_type_system->GetDWARFParser();<br>
+ if (!dwarf_ast)<br>
+ return TypeSP();<br>
+<br>
+ Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);<br>
+ type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log, type_is_new_ptr);<br>
+ if (!type_sp)<br>
+ return TypeSP();<br>
+<br>
+ TypeList *type_list = GetTypeList();<br>
+ if (type_list)<br>
+ type_list->Insert(type_sp);<br>
----------------<br>
revert<br>
<br>
================<br>
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:246<br>
@@ -245,3 +245,2 @@<br>
<br>
-<br>
//------------------------------------------------------------------<br>
----------------<br>
Revert. Whitespace only change<br>
<br>
================<br>
Comment at: source/Symbol/GoASTContext.cpp:1642-1649<br>
@@ -1641,10 +1641,2 @@<br>
<br>
-DWARFASTParser *<br>
-GoASTContext::GetDWARFParser()<br>
-{<br>
- if (!m_dwarf_ast_parser_ap)<br>
- m_dwarf_ast_parser_ap.reset(new DWARFASTParserGo(*this));<br>
- return m_dwarf_ast_parser_ap.get();<br>
-}<br>
-<br>
UserExpression *<br>
----------------<br>
revert<br>
<br>
================<br>
Comment at: source/Symbol/JavaASTContext.cpp:499-506<br>
@@ -498,10 +498,2 @@<br>
<br>
-DWARFASTParser *<br>
-JavaASTContext::GetDWARFParser()<br>
-{<br>
- if (!m_dwarf_ast_parser_ap)<br>
- m_dwarf_ast_parser_ap.reset(new DWARFASTParserJava(*this));<br>
- return m_dwarf_ast_parser_ap.get();<br>
-}<br>
-<br>
ConstString<br>
----------------<br>
revert<br>
<br>
<br>
<a href="http://reviews.llvm.org/D18381" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18381</a><br>
<br>
<br>
<br>
</blockquote></div>