[Lldb-commits] [PATCH] D18381: Decouple ClangASTContext from DWARF

[Zachary Turner via lldb-commits]

zturner added inline comments.

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1614-1622
@@ -1611,9 +1613,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;
+
+    DWARFASTParser *ast_parser = clang_type_system->GetDWARFParser();
+    if (!ast_parser)
+        return false;
+
+    return ast_parser->CanCompleteType(compiler_type);
 }
----------------
clayborg wrote:
> This can be reverted since GetDWARFParser is stying in TypeSystem
Ahh, you've actually alerted me to a subtle bug.  `CanCompleteType` and `CompleteType` are not even implemented by `DWARFASTParserClang` anymore, because they were just forwards into the `ClangASTImporter`.  So instead people are expected to call `ast_parser->GetClangASTImporter().CanImport()` or `ast_parser->GetClangASTImporter().Import()`.  So the code here is actually calling the `CanCompleteType` method in the base class `DWARFASTParser` which simply returns false.  

So I'll need to change this to delete the empty implementations from the base class, and call straight into the `ClangASTImporter`.


http://reviews.llvm.org/D18381