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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
