llunak added a comment. In D122974#3538413 <https://reviews.llvm.org/D122974#3538413>, @dblaikie wrote:
> In D122974#3536342 <https://reviews.llvm.org/D122974#3536342>, @llunak wrote: > >> D124704 <https://reviews.llvm.org/D124704> adds a unittest that compares >> StringMap::hash() to a known hardcoded value, so whenever the hash >> implementation changes, it won't be possible to unittest LLDB with that >> change, and that will be the time to change the lldb cache version. > > Ah, good stuff - doesn't guarantee that any hash change necessarily breaks > the test, but certainly helps/seems like a good idea, thanks! I think that does guarantee it in practice. If changing a string hash implementation does not change the result for a randomly chosen non-trivial input, then either the implementation is compatible or it's very poor, so I don't see why either of these should be a practical concern. But if the highly improbable case is a concern for some reason I fail to see, I can make the test check two different inputs, which should make the case virtually impossible. >> If the theory is that this should keep working even with the library >> changing without LLDB rebuild, then as I wrote above that theory needs >> double-checking first. And additionally a good question to ask would be if >> it's really a good idea to do incompatible implementation changes to a class >> that has half of its functionality inline. > > Sorry, I wasn't meaning to discuss dynamic library versioning > issues/mismatches, just static/fully matched versions. Then I still don't know what the problem is supposed to be. If the StringMap hash implementation ever changes, the necessary LLDB rebuild will detect this, the relevant LLDB parts will get adjusted and problem solved. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122974/new/ https://reviews.llvm.org/D122974 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
