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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: include/lldb/Symbol/ClangASTContext.h:895
 
+  /// Dump clang AST types from the symbol table
+  ///
----------------
nit: `.` at the end


================
Comment at: include/lldb/Symbol/ClangASTContext.h:896
+  /// Dump clang AST types from the symbol table
+  ///
+  /// \param[in] s
----------------
What is "the symbol table" in this context? Does ClangASTContext have access to one?


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3059
+
+    if (isType(dwarf_tag) && tag != DW_TAG_subrange_type)
+      ParseType(sc, die, &type_is_new);
----------------
comment explaining why not subrange type?


================
Comment at: source/Symbol/ClangASTContext.cpp:9166
 
+void ClangASTContext::DumpFromSymbols(Stream &s, llvm::StringRef symbol_name) {
+  SymbolFile *symfile = GetSymbolFile();
----------------
Ah .. so should it be called DumpFromSymbolFile() or LookupAndDump()?



================
Comment at: source/Symbol/ClangASTContext.cpp:9173
+  lldb_private::TypeList type_list;
+  size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+
----------------
This API with the return value doesn't exit any more.


================
Comment at: source/Symbol/ClangASTContext.cpp:9182
+
+    s << type->GetName().AsCString() << "\n";
+
----------------
the AsCString() part seems to be bad for the performance.. is there a variant that returns a StringRef instead?


================
Comment at: source/Symbol/ClangASTContext.cpp:9184
+
+    if (clang::CXXRecordDecl *record_decl =
+            GetAsCXXRecordDecl(type->GetFullCompilerType().GetOpaqueQualType()))
----------------
A switch over the kind would be more efficient and potentially nicer to read?


================
Comment at: source/Symbol/ClangASTContext.cpp:9195
+                 GetAsEnumDecl(type->GetFullCompilerType()))
+      enum_decl->dump(s.AsRawOstream());
+    else {
----------------
is an EnumDecl not a TagDecl?


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

https://reviews.llvm.org/D67994