aprantl abandoned this revision. aprantl marked an inline comment as done. aprantl added inline comments.
================ Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:996 m_clang_ast_context->GetUniqueNamespaceDeclaration( - g_lldb_local_vars_namespace_cstr, nullptr); + g_lldb_local_vars_namespace_cstr, nullptr, 0); if (!namespace_decl) ---------------- shafik wrote: > We are passing around `0` everywhere and it is not obvious what it means at > all. > > Can we name this somehow or make it a type? This is just an intermediate step and gets replaced in https://reviews.llvm.org/D75488 ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:146 DWARFDIE *decl_ctx_die); + lldb_private::ModuleID GetOwningModuleID(const DWARFDIE &die); ---------------- labath wrote: > I am kinda lost at this point, so I'm not sure which patch introduces this, > but... this name does not seem very ideal, as it sounds like it is related to > `lldb_private::Module`, while it does not really have that much in common > with it. > > And then there's `ClangModulesDeclVendor::ModuleID` -- I have no idea what's > the relationship of this to that.. I agree. I'm going to clean this up some more and I will also merge this review into https://reviews.llvm.org/D75488 because it doesn't actually make reviewing easier to see these API changes without their use. ================ Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h:73 + unsigned GetOwningModuleID() { return Flags(m_payload).Clear(ObjCClassBit); } + void SetOwningModuleID(unsigned id) { + assert(id < ObjCClassBit); ---------------- shafik wrote: > Why not use `uint32_t` like we did above? If we are going to assume `32 bits` > we should just use the fixed width type. That is consistent with the type Clang uses for module IDs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75626/new/ https://reviews.llvm.org/D75626 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits