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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to