Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code [v2]

2022-06-01 Thread Naoto Sato
On Wed, 1 Jun 2022 04:10:03 GMT, ExE Boss  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Removed unnecessary clone() call
>
> src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java line 
> 181:
> 
>> 179: .toArray(Locale[]::new);
>> 180: }
>> 181: return AVAILABLE_LOCALES;
> 
> This should probably clone the cached array:
> Suggestion:
> 
> return AVAILABLE_LOCALES.clone();
> 
> 
> Matching what `JRELocaleProviderAdapter` does[^1], since there’s no guarantee 
> that the result of `getAvailableLocales()` won’t be mutated.
> 
> [^1]: 
> https://github.com/openjdk/jdk/blob/6b1169e266b9d21864f886ef574dd64116fa2cb0/src/java.base/share/classes/sun/util/locale/provider/JRELocaleProviderAdapter.java#L430-L439

Thanks for your comments. In fact, `clone()` in `JRELocaleProviderAdapter` is 
not necessary, as it will be subsumed into `Locale.getAvailableLocales()` and 
other `getAvailableLocales()` methods in locale sensitives services (such as 
`DateFormat`) where they are defensively cloned. Removed this `clone()` 
invocation.

-

PR: https://git.openjdk.java.net/jdk/pull/8960


Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code [v2]

2022-06-01 Thread Naoto Sato
> Refactoring some old code in locale providers. The test case data have also 
> been modified due to:
> - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not 
> handle the case for `no-NO-NY`.
> - `Locale.toLanguageTag()` won't handle legacy Java locales, e.g., `ja_JP_JP` 
> and falls back, so comparing locales using language tags does not work for 
> those locales. Changed to compare with `Locale.toString()` instead.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Removed unnecessary clone() call

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8960/files
  - new: https://git.openjdk.java.net/jdk/pull/8960/files/6b1169e2..d1b00f10

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8960=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8960=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8960.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8960/head:pull/8960

PR: https://git.openjdk.java.net/jdk/pull/8960