Change lldb-test to use ParseAllDebugSymbols.

ParseDeclsForContext was originally created to serve the very specific
case where the context is a function block. It was never intended to be
used for arbitrary DeclContexts, however due to the generic name, the
DWARF and PDB plugins implemented it in this way "just in case". Then,
lldb-test came along and decided to use it in that way.

Related to this, there are a set of functions in the SymbolFile class
interface whose requirements and expectations are not documented. For
example, if you call ParseCompileUnitFunctions, there's an inherent
requirement that you create entries in the underlying clang AST for
these functions as well as their signature types, because in order to
create an lldb_private::Function object, you have to pass it a
CompilerType for the parameter representing the signature.

On the other hand, there is no similar requirement (either inherent or
documented) if one were to call ParseDeclsForContext. Specifically, if
one calls ParseDeclsForContext, and some variable declarations, types,
and other things are added to the clang AST, is it necessary to create
lldb::Variable, lldb::Type, etc objects representing them? Nobody knows.
There is, however, an accidental requirement, because since all of the
plugins implemented this just in case, lldb-test came along and used
ParsedDeclsForContext, and then wrote check lines that depended on this.

When I went to try and implemented the NativePDB reader, I did not
adhere to this (in fact, from a layering perspective I went out of my
way to avoid it), and as a result the existing DIA PDB tests don't work
when the native PDB reader is enabled, because they expect that calling
ParseDeclsForContext will modify the *module's* view of symbols, and not
just the internal AST.

All of this confusion, however, can be avoided if we simply stick to
using ParseDeclsForContext for its original intended use case (blocks),
and use a different function (ParseAllDebugSymbols) for its intended use
case which is, unsuprisingly, to parse all the debug symbols (which is
all lldb-test really wanted to do anyway).

In the future, I would like to change ParseDeclsForContext to
ParseDeclsForFunctionBlock, then delete all of the dead code inside that
handles other types of DeclContexts (and probably even assert if the
DeclContext is anything other than a block).

A few PDB tests needed to be fixed up as a result of this, and this also
exposed a couple of bugs in the DIA PDB reader (doesn't matter much
since it should be going away soon, but worth mentioning) where the
appropriate AST entries weren't being created always.

Differential Revision: https://reviews.llvm.org/D56418

1 files changed, 1 insertions, 3 deletions

diff --git a/lldb/tools/lldb-test/lldb-test.cpp b/lldb/tools/lldb-test/lldb-test.cpp
index 62abc42979d..4e2ab504c2d 100644
--- a/lldb/tools/lldb-test/lldb-test.cpp
+++ b/lldb/tools/lldb-test/lldb-test.cpp
@@ -518,6 +518,7 @@ Error opts::symbols::dumpModule(lldb_private::Module &Module) {
 
 Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   SymbolVendor &plugin = *Module.GetSymbolVendor();
+   Module.ParseAllDebugSymbols();
 
   auto symfile = plugin.GetSymbolFile();
   if (!symfile)
@@ -536,9 +537,6 @@ Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   if (!tu)
     return make_string_error("Can't retrieve translation unit declaration.");
 
-  symfile->ParseDeclsForContext(CompilerDeclContext(
-      clang_ast_ctx, static_cast<clang::DeclContext *>(tu)));
-
   tu->print(outs());
 
   return Error::success();