labath added a comment. It's not clear to me how much of this patch is pure refactoring, and how much of it is adding new features. It would have been better to split that out into two patches.
I see some layout handling code in `UdtRecordCompleter` constructor, but that's just two lines of code. Is that it, or should I look elsewhere? ================ Comment at: lldb/include/lldb/Symbol/TypeSystem.h:31-32 class DWARFASTParser; class PDBASTParser; +class PdbAstBuilder; ---------------- Uh-oh. This is definitely not an intuitive naming scheme. How about we keep this class in the `lldb_private::npdb` namespace? You can forward-declare it there just as well. Layering-wise, this code here is pretty bad, but moving the parser declaration into the global namespace is not going to make that better. And then you can undo all of the namespacing churn in the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134509/new/ https://reviews.llvm.org/D134509 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
