On Sun, 25 Jun 2023 18:42:14 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
> I'll re-run our CI, and if all good, I'll sponsor this PR. The CI tests I started have just passed. While this PR is already good, I wonder if we make it even better. I doubt highly that we need null-checks for CacheKey's name and locale. My suspicion comes from these two facts: * CacheKey's hashCode calls methods on these fields without checking them for null, and * CacheKey instances are queried and stored in ConcurrentHashMap, which certainly uses both `equals` and `hashCode` So, what I'm saying is that if those fields were null, there would be unhandled NPE somewhere; but I cannot see such NPE. Additionally, those fields are used in toString(). Furthermore, locale is known to not be null because the only place from where it is passed to a CacheKey consturctor is ResourceBundle.getBundleImpl(Module, Module, String, Locale, ResourceBundle.Control), which explicitly checks that locale for not being null. Does it make sense? ------------- PR Comment: https://git.openjdk.org/jdk/pull/12328#issuecomment-1606291015
