clayborg added a comment. In D77790#1974047 <https://reviews.llvm.org/D77790#1974047>, @jarin wrote:
> Regarding the callback idea, I have bad experience with callbacks because > they break if the code is not crafted for re-entrancy and they are much > harder to understand. That change feels out of scope for just adding a test > and fixing an unrelated bug. The code is already there and working. Not sure what the worry about re-entrancy is from? > Adding the SetCacheLineSize method sounds good, but if we want to keep this > patch making NFC (which is what I would prefer), it would have to be called > at exactly the same places as Clear. How about Pavel's idea to rename Clear > -> Reset, and leave the refactoring to SetCacheLineSize for later? We are changing the code so that just to make it NFC we have to check it in some half baked state? I don't agree with this. Clear() didn't take a parameter before, nor should it have to now. > It appears it is really hard to reach agreement about this, so another > alternative is I submit a bug report about the L1 > <https://reviews.llvm.org/L1> invalidation problem and leave it to you to > figure this out. In the mean time, we will fix the bug only in our private > fork of lldb. Greg, perhaps you would prefer that? I don't see this as really that hard to fix correctly. Feel free to do what you need to if this is too much. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77790/new/ https://reviews.llvm.org/D77790 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
