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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for splitting this up. We still need to figure out what to do with the first patch, but these two are looking very good now.

At least, in the sense that one can clearly see what is happening -- I'd still like to have to discussion about the DIERef->user_id conversion. Essentially, I'd like to see if we can preserve the property that the conversion always produces a valid user_id. With this patch that is not true anymore, because the OSO DIERefs need to be passed through the SymbolFileDWARF::GetUID function, even though it is _very_ tempting to just call `get_id()` on them. Previously, there was no get_id function, so one had no choice but to call GetUID. If we can make sure that the OSO symbol files always construct DIERefs with the oso field populated correctly, then I think this approach would be fine (and we should be able to delete the GetUID function). Otherwise, I'd like to explore some options of keeping the DIERef->user_id conversion tightly controlled. Personally, I am still not convinced that doing the conversion in the GetUID function is a bad thing. IIUC, the main problem is the part where we do a bitwise or of the DIERef (the die offset part) and the OSO ID (`GetID() | ref.die_offset()`), but I don't see why we couldn't do something about that. Basically we could just create an inverse function of `GetOSOIndexFromUserID` (which extracts the OSO symbol file from the user id) -- and make sure the functions are close to each other and their implementation matches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618