[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

Tue Jan 24 10:31:38 PST 2023

ayermolo added a comment.

In D138618#4073329 <https://reviews.llvm.org/D138618#4073329>, @labath wrote:

> 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?

Searching for GetUID doesn't look like it's used all that often, maybe follow up patch is just to get rid of it, and replace with DIERef?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618