[Lldb-commits] [PATCH] D89157: [lldb] Report old modules from ModuleList::ReplaceEquivalent

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D89157#2362697 <https://reviews.llvm.org/D89157#2362697>, @JosephTremoulet wrote:

>> But then you get to a point where you shouldn't really have multiple modules replacing a single one so you aren't really sure what to do about it. That part makes me a little uneasy.
>
> Yeah, I wouldn't claim that the handling of the multiple-old-module case there is ideal. My thinking is that it makes an incremental step in the right direction, though -- the same potential for having multiple old modules is there with or without the change; the change makes the issue more apparent to readers of the code, makes note of it in the relevant log if we hit the issue at runtime, and refactors the code so there is one place to handle the issue. If you think it's best to hold off on that until we have a better way to actually handle the case, I can see the logic in that, just let me know and I'll rebase this to fix the bug in the meantime.
>
> Thanks!

Since we'd just be dropping all the other modules that we don't know how they are coming about on the floor before your patch, that seems strictly better.  Can you emit more info when this happens, like the module that's replacing this set of old modules, and the UUID of the duplicate entries.  That way when this happens for some reason we didn't expect - which will invariably be on some proprietary code that we can't debug locally - we have a better chance of figuring out what went wrong.  Then this should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89157