ayermolo added a comment. In D138618#4102275 <https://reviews.llvm.org/D138618#4102275>, @labath wrote:
> In D138618#4094933 <https://reviews.llvm.org/D138618#4094933>, @clayborg > wrote: > >>> - My main source of frustration was that my concern is getting >>> overlooked/ignored (not necessarily your fault -- I've been told I am not >>> always sufficiently clear). I think that is something we could live with, >>> if we thing the other cleanups in this patch are worth it (which could very >>> well be the case) -- however, I would want us to be clear that's what we're >>> doing. >> >> I do want to state that if we fix things the way I describe it will work >> seamlessly with OSO or DWO files. The current state of things is the DWO >> stuff only uses the fancy DIERef constructor and fills in the dwo number >> correctly only to have it overwritten in SymbolFile::GetUID(...). The >> SymbolFile::GetUID(...) is needed for OSOs currently because the DIERef that >> SymbolFileDWARF (which is used for OSO) doesn't correctly create DIERef >> objects since they always return llvm::None for >> SymbolFileDWARF::GetDwoNum(). But the new API will have >> SymbolFileDWARF::GetFileIndex() to be used instead of >> SymbolFileDWARF::GetDwoNum(), and the file index will be set correctly for >> both DWO and OSO files. We can then change DIERef away from DWO specific >> naming, and have DIERef have a "m_file_index" and "m_file_index_valid" >> instead of the dwo specific members. As long as both OSO and DWO files can >> be found from the user_id_t API calls we are all good. Not sure if this >> addresses all of your issues or not. >> >> If all of your concerns are not clarified above, can you clarify what is >> still being overlooked? Both Alexander and I are usually thinking we are >> addressing everything you want, but we obviously still aren't, so restating >> your remaining concerns would help us get this patch moving. > > Everything is clarified is and fine. I agree with your plan Thanks. I was > trying to return the favor and clarify my own (potentially rude) responses. Great, thank you for your and Greg feedback. Let me modify my changes according to Gregs suggestion. 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
