[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 7 inline comments as done.
shafik added a comment.

I ended up splitting out the functionality into a new method `dumpClangAST` it looks PDB is not as lazy as DWARF and there are several PDB tests already using the current `dumpAST` as it is.



================
Comment at: tools/lldb-test/lldb-test.cpp:552-579
+    
+    lldb_private::TypeList type_list;
+    size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+    printf( "Type list size: %zu\n", ntypes);
+    
+    for( size_t i = 0; i < ntypes; ++i) {
+        auto type = type_list.GetTypeAtIndex(i);
----------------
shafik wrote:
> clayborg wrote:
> > I know that there is already clang AST stuff in this file, but it seems like anything in this file should be just passed to the type system for dumping? This code could be:
> > 
> > ```
> > lldb_private::TypeList type_list;
> > size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
> > printf( "Type list size: %zu\n", ntypes);
> > for( size_t i = 0; i < ntypes; ++i)
> >   type_list.GetTypeAtIndex(i)->DumpTypeValue(...);
> > ```
> > 
> > Better yet this entire function could just become:
> > 
> > ```
> > Error opts::symbols::dumpAST(lldb_private::Module &Module) {
> >   Module.ParseAllDebugSymbols();
> > 
> >   auto symfile = Module.GetSymbolFile();
> >   if (!symfile)
> >     return make_string_error("Module has no symbol file.");
> > 
> >   auto type_system_or_err =
> >       symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
> >   if (type_system_or_err)
> >     type_system_or_err->DumpAST(...);
> >   else
> >     return make_string_error("Can't retrieve TypeSystem");
> > }
> > ```
> > And all clang AST specific stuff can be removed from this binary? Tests would need to be updated.
> > 
> If we do stick with this approach pushing the `DumpAST(...)` into `TypeSystem` seems reasonable.
It looked like pushing this into `ClangASTContext` made more sense. I am happy to bikeshed naming though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67994/new/

https://reviews.llvm.org/D67994