[Lldb-commits] [PATCH] D18381: Decouple ClangASTContext from DWARF
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 22 17:03:28 PDT 2016
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.
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.
One option is to add back the SymbolFormat enum and make a method in
TypeSystem like
DebugInfoASTParser *
GetASTParserForDebugFormat(SymbolFormat format)
This way it's not DWARF specific.
Another option is to add a method for pdb and a method for DWARF. I think
the former is more flexible though.
On Tue, Mar 22, 2016 at 4:22 PM Greg Clayton <clayborg at gmail.com> wrote:
> clayborg requested changes to this revision.
> clayborg added a comment.
> This revision now requires changes to proceed.
>
> 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.
>
> 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..
>
>
> ================
> Comment at: include/lldb/Symbol/ClangASTContext.h:37-39
> @@ -37,2 +36,5 @@
> #include "lldb/Symbol/TypeSystem.h"
> +#include "lldb/lldb-enumerations.h"
> +
> +class DWARFASTParserClang;
>
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/ClangASTContext.h:69-71
> @@ -66,2 +68,5 @@
>
> + DWARFASTParserClang *
> + GetDWARFParser();
> +
> //------------------------------------------------------------------
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/ClangASTContext.h:521-526
> @@ -520,8 +525,2 @@
> //------------------------------------------------------------------
> - // TypeSystem methods
> - //------------------------------------------------------------------
> - DWARFASTParser *
> - GetDWARFParser () override;
> -
> - //------------------------------------------------------------------
> // ClangASTContext callbacks for external source lookups.
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/ClangASTContext.h:1228
> @@ -1227,3 +1227,3 @@
> std::unique_ptr<clang::Builtin::Context> m_builtins_ap;
> - std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;
> + std::unique_ptr<DWARFASTParserClang> m_dwarf_ast_parser_ap;
> std::unique_ptr<ClangASTSource>
> m_scratch_ast_source_ap;
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/GoASTContext.h:63-64
> @@ -62,4 +62,2 @@
> -
> - DWARFASTParser *GetDWARFParser() override;
>
> void
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/GoASTContext.h:390
> @@ -389,3 +387,2 @@
> std::unique_ptr<TypeMap> m_types;
> - std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;
>
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/JavaASTContext.h:61-63
> @@ -60,5 +60,2 @@
>
> - DWARFASTParser *
> - GetDWARFParser() override;
> -
> uint32_t
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/JavaASTContext.h:371
> @@ -370,3 +367,2 @@
> uint32_t m_pointer_byte_size;
> - std::unique_ptr<DWARFASTParser> m_dwarf_ast_parser_ap;
> JavaTypeMap m_array_type_map;
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/TypeSystem.h:31
> @@ -30,3 +30,1 @@
>
> -class DWARFDIE;
> -class DWARFASTParser;
> ----------------
> This line can still go as it isn't needed.
>
> ================
> Comment at: include/lldb/Symbol/TypeSystem.h:32
> @@ -31,3 @@
> -class DWARFDIE;
> -class DWARFASTParser;
> -
> ----------------
> 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.
>
> ================
> Comment at: include/lldb/Symbol/TypeSystem.h:102-107
> @@ -101,8 +98,2 @@
>
> - virtual DWARFASTParser *
> - GetDWARFParser ()
> - {
> - return nullptr;
> - }
> -
> virtual SymbolFile *
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:12-30
> @@ -11,17 +11,21 @@
>
> +#include "DWARFASTParser.h"
> #include "DWARFCompileUnit.h"
> +#include "DWARFDIECollection.h"
> #include "DWARFDebugAbbrev.h"
> #include "DWARFDebugAranges.h"
> #include "DWARFDebugInfo.h"
> #include "DWARFDebugInfoEntry.h"
> #include "DWARFDebugRanges.h"
> #include "DWARFDeclContext.h"
> -#include "DWARFDIECollection.h"
> #include "DWARFFormValue.h"
> #include "SymbolFileDWARF.h"
>
> #include "lldb/Core/Module.h"
> +#include "lldb/Symbol/ClangASTContext.h"
> #include "lldb/Symbol/ObjectFile.h"
> #include "lldb/Symbol/Type.h"
> #include "lldb/Symbol/TypeSystem.h"
>
> +#include "Plugins\SymbolFile\DWARF\DWARFASTParserClang.h"
> +
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:408-415
> @@ -403,11 +407,10 @@
>
> DWARFASTParser *
> DWARFDIE::GetDWARFParser () const
> {
> - lldb_private::TypeSystem *type_system = GetTypeSystem ();
> - if (type_system)
> - return type_system->GetDWARFParser();
> - else
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(GetTypeSystem());
> + if (!clang_type_system)
> return nullptr;
> + return clang_type_system->GetDWARFParser();
> }
>
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:57
> @@ -56,2 +56,3 @@
> #include "DWARFASTParser.h"
> +#include "DWARFASTParserClang.h"
> #include "DWARFCompileUnit.h"
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:997-1000
> @@ -995,6 +996,6 @@
> TypeSystem *type_system =
> GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
> -
> - if (type_system)
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(type_system);
> + if (clang_type_system)
> {
> - DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
> + DWARFASTParser *dwarf_ast =
> clang_type_system->GetDWARFParser();
> if (dwarf_ast)
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1465-1469
> @@ -1463,3 +1464,7 @@
> TypeSystem *type_system = decl_ctx.GetTypeSystem();
> - DWARFASTParser *ast_parser = type_system->GetDWARFParser();
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(type_system);
> + if (!clang_type_system)
> + return;
> +
> + DWARFASTParser *ast_parser = clang_type_system->GetDWARFParser();
> std::vector<DWARFDIE> decl_ctx_die_list =
> ast_parser->GetDIEForDeclContext(decl_ctx);
> ----------------
> 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.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1617-1625
> @@ -1611,9 +1616,11 @@
> TypeSystem *type_system = compiler_type.GetTypeSystem();
> - if (type_system)
> - {
> - DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
> - if (dwarf_ast)
> - return dwarf_ast->CanCompleteType(compiler_type);
> - }
> - return false;
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(type_system);
> + if (!clang_type_system)
> + return false;
> +
> + DWARFASTParserClang *ast_parser = clang_type_system->GetDWARFParser();
> + if (!ast_parser)
> + return false;
> +
> + return ast_parser->CanCompleteType(compiler_type);
> }
> ----------------
> Revert unless you need to route this to your new Clang external source
> changes you made.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1635-1638
> @@ -1627,5 +1634,6 @@
> TypeSystem *type_system = compiler_type.GetTypeSystem();
> - if (type_system)
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(type_system);
> + if (clang_type_system)
> {
> - DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
> + DWARFASTParserClang *dwarf_ast =
> clang_type_system->GetDWARFParser();
> if (dwarf_ast && dwarf_ast->CanCompleteType(compiler_type))
> ----------------
> revert.
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3828-3846
> @@ -3819,6 +3827,21 @@
>
> - if (die)
> - {
> - TypeSystem *type_system =
> GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
> + if (!die)
> + return TypeSP();
>
> - if (type_system)
> + TypeSystem *type_system =
> GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
> + ClangASTContext *clang_type_system =
> llvm::dyn_cast_or_null<ClangASTContext>(type_system);
> + if (!clang_type_system)
> + return TypeSP();
> +
> + DWARFASTParser *dwarf_ast = clang_type_system->GetDWARFParser();
> + if (!dwarf_ast)
> + return TypeSP();
> +
> + Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
> + type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log,
> type_is_new_ptr);
> + if (!type_sp)
> + return TypeSP();
> +
> + TypeList *type_list = GetTypeList();
> + if (type_list)
> + type_list->Insert(type_sp);
> ----------------
> revert
>
> ================
> Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:246
> @@ -245,3 +245,2 @@
>
> -
> //------------------------------------------------------------------
> ----------------
> Revert. Whitespace only change
>
> ================
> Comment at: source/Symbol/GoASTContext.cpp:1642-1649
> @@ -1641,10 +1641,2 @@
>
> -DWARFASTParser *
> -GoASTContext::GetDWARFParser()
> -{
> - if (!m_dwarf_ast_parser_ap)
> - m_dwarf_ast_parser_ap.reset(new DWARFASTParserGo(*this));
> - return m_dwarf_ast_parser_ap.get();
> -}
> -
> UserExpression *
> ----------------
> revert
>
> ================
> Comment at: source/Symbol/JavaASTContext.cpp:499-506
> @@ -498,10 +498,2 @@
>
> -DWARFASTParser *
> -JavaASTContext::GetDWARFParser()
> -{
> - if (!m_dwarf_ast_parser_ap)
> - m_dwarf_ast_parser_ap.reset(new DWARFASTParserJava(*this));
> - return m_dwarf_ast_parser_ap.get();
> -}
> -
> ConstString
> ----------------
> revert
>
>
> http://reviews.llvm.org/D18381
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160323/d941f9b8/attachment-0001.html>
More information about the lldb-commits
mailing list