On Fri, 1 May 2026 16:58:04 GMT, Justin Lu <[email protected]> wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Removed unnecessary throws > > src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java > line 660: > >> 658: var inputSkeletons = new HashMap<String, Map<String, String>>(); >> 659: Pattern p = Pattern.compile("([^:]+):([^;]+);"); >> 660: if >> (LocaleProviderAdapter.forType(LocaleProviderAdapter.Type.CLDR) instanceof >> ResourceBundleBasedAdapter rbba) { > > Since the new code relies on matching the results of > `LocaleProviderAdapter.forType(LocaleProviderAdapter.Type.CLDR)`, I think > there should be a reasonable `InternalError` or the sorts in an `else` block > (or perhaps write it as an inverted `if`). Writing it this way makes it clear > to the reader that returning an empty map is not a possible intentional path > and also ensures an empty map isn't silently returned by the method in the > case of an error (even if unlikely).
Since input skeletons are not mandatory elements, theoretically it could be possible that CLDR data do not include it (very unlikely though). So instead of throwing an error here, defaulting to "h" at the calling site, even if the map is empty, would be desirable. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31006#discussion_r3175023874
