labath added a comment.
In D138618#4060565 <https://reviews.llvm.org/D138618#4060565>, @clayborg wrote:
> ...
> Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else,
> we can make them what we need them to be so they work for us. I would suggest
> to remove the use of DIERef from calculating the IDs of symbol files and have
> .o files for mac and .dwo files for fission use a 1 based index as their ID
> to make it easy to encode into a DIERef when needed for lldb::user_id_t
> values that _are_ included in objects that we hand out. Is there anything
> else that would need to be done to keep everyone happy here?
I think that the 1-based index thingy helps a lot here, but I haven't seen
anything (in your reponse, or in the new patch) that would address my concernt
DIERef<->user_id conversion ambiguity. I.e. how is one supposed to know what is
the "right" way to convert a DIERef to a user_id:
- `die_ref.get_id()`
- or `symbol_file.GetUID(die_ref)` (which, funnily enough, will construct
another DIERef, and *then* call get_id? (`return DIERef(GetID(), ref.section(),
ref.die_offset()).get_id();`)
What's your position on that? That we should live with the ambiguity?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:53
+
+ void set_die_offset(dw_offset_t offset) {
+ lldbassert(offset <= DW_INVALID_OFFSET);
----------------
Can we remove this function now?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:11
+#include "DIERef.h"
#include "lldb/Core/Section.h"
----------------
nor this
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h:12
+#include "DIERef.h"
#include "SymbolFileDWARF.h"
----------------
I guess this isn't necessary anymore.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138618/new/
https://reviews.llvm.org/D138618
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
