labath added a comment. 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. 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
