clayborg added a comment. In D138618#4080706 <https://reviews.llvm.org/D138618#4080706>, @labath wrote:
> In D138618#4077851 <https://reviews.llvm.org/D138618#4077851>, @ayermolo > wrote: > >> In D138618#4073329 <https://reviews.llvm.org/D138618#4073329>, @labath wrote: >> >>> 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? > > If you could make that work, that would be awesome, but I think that's going > to be fairly hard. It's true that there aren't that many call sites of this > functions, but the ones that are there are very crucial. The user_id_t type > represents a symbol-file-neutral identifier (cookie, if you will) that > different symbol file implementations use to identify parsed objects (types, > mostly). SymbolFileDWARF uses it (via DIERef et al.) to identify the DIE > belonging to that type. PDB symbol files use it differently, but the idea is > the same. If we wanted to remove that, we'd have to come up with a whole new > way to parse/link types -- and one that would work for non-dwarf symbol files > as well. SymbolFileDWARF is the only SymbolFile that inherits from UserID. So this is a DWARF internal thing only where symbol files have user_id_t values. So as long as we make this work for all things DWARF we are good to go. > (also, this would not really address my concern, because the question would > then become "which DIERef is safe to be used as a Type cookie" (answer: only > the one which has the OSO field set), but the reduction in complexity > resulting from removing one step from the conversion process > (DWARFDIE->DIERef->user_id) might be well worth it.) DIERef should be used anywhere a DIE needs a user_id_t. If DIERef now uses the file index (OSO index, or DWO index) in the same way where we always ask the SymbolFileDWARFXXX::GetID() as the file index, then things will be the same between the two (DWO or OSO). 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
