labath added a comment.
In D138618#4083481 <https://reviews.llvm.org/D138618#4083481>, @clayborg wrote:
> We just need to create all DIERef objects using the GetID() from the symbol
> file as the file index, and we should be able to remove the
> SymbolFile::GetUID() function now. As long as file index zero is reserved for
> "vanilla DWARF that doesn't use DWO or OSO we will know the difference. We
> might want to not have SymbolFileDWARF inherit from UserID at all, and switch
> over to have SymbolFileDWARF add a virtual function:
>
> uint32_t m_file_index = 0; // Zero means main DWARF file, 1...N identifies
> the Nth DWO file or OSO file
> virtual uint32_t GetFileIndex() { return m_file_index; }
>
> Then anyone can set the file index correctly for DWO or OSO files. And we
> avoid using user_id_t values for the symbol files since they aren't needed.
This isn't about the "user id" of a symbol file. I'm totally happy with the
changes there -- though I also wouldn't be opposed to changing the "user id"
field to something more explicit (like the file index).
My problem is with the "user id"s of individual DIEs. Currently, if I have a
DWARFDIE, the only way to get its user id is to do something like
`die.GetDWARF()->GetUID(die)`. With this patch, there are two ways:
1. the same as before
2. `die.GetDIERef()->get_id()`
The problem is that the second way is not going to be correct for OSO files
because that path will not set the "oso" component of the DIERef. The worst
part is that the second method is much shorter than the first one, so I think
it will be very tempting to use it -- and it will actually be right most of the
time, until that code is used in an OSO context.
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
