RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern

[Jorn Vernee]

On Tue, 28 Sep 2021 13:28:24 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>>> FWIW, there was some prior discussion here about this code as well: #3107
>>> 
>>> tl;dr MSVC uses the system locale's code page to parse this, which must be set to `en-us` to work. From the fact that you are getting these warnings, I guess you have a different locale set (can check with `systeminfo`). See also: https://bugs.openjdk.java.net/browse/JDK-8264425
>>> 
>>> Though, I can't comment on the changes in this PR.
>> 
>> Thanks @JornVernee for your comments.
>> 
>> My system local is zh-cn.
>> 
>> But changing the locale isn't acceptable since many of our Apps require zh-cn in our country.
>> 
>> According to the JBS, C4819 warning was first observed with VS2017 and was disabled by JDK-8216154.
>> Then C4474 and C4778 warnings are triggered with VS2019.
>> So the code is actually non-portable.
>> 
>> If the non-ascii code is useless, it should be removed to make HotSpot to be more portable.
>> Then C4819, C4474 and C4778 would disappear.
>> And OpenJDK can be built without changing the locale setting. 
>> 
>> Thanks.
>
>> My system local is zh-cn.
>> 
>> But changing the locale isn't acceptable since many of our Apps require zh-cn in our country.
> 
> I understand, and that is totally reasonable to me.
> 
> There might be another way to change the locale just for the compilation [1], but I haven't had time to test that (so for now I think the official advice is to us `en-us` as a system locale). I think something like this should also remove the warnings (after `reconfigure clean`):
> 
> 
> diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4
> index ba4cb3e9ac7..3078a2e916c 100644
> --- a/make/autoconf/flags-cflags.m4
> +++ b/make/autoconf/flags-cflags.m4
> @@ -496,7 +496,7 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
>      TOOLCHAIN_CFLAGS_JVM="-qtbtable=full -qtune=balanced \
>          -qalias=noansi -qstrict -qtls=default -qnortti -qnoeh -qignerrno -qstackprotect"
>    elif test "x$TOOLCHAIN_TYPE" = xmicrosoft; then
> -    TOOLCHAIN_CFLAGS_JVM="-nologo -MD -MP"
> +    TOOLCHAIN_CFLAGS_JVM="-nologo -MD -MP -utf-8"
>      TOOLCHAIN_CFLAGS_JDK="-nologo -MD -Zc:wchar_t-"
>    fi
> 
> 
> 
> In your case the compiler produced some warnings, but I'm wondering if using a different encoding could also silently create subtle behavioral changes. I think it would be good if a specific encoding could be used at build time.
>  
>> According to the JBS, C4819 warning was first observed with VS2017 and was disabled by JDK-8216154.
>> Then C4474 and C4778 warnings are triggered with VS2019.
>> So the code is actually non-portable.
>> 
>> If the non-ascii code is useless, it should be removed to make HotSpot to be more portable.
>> Then C4819, C4474 and C4778 would disappear.
>> And OpenJDK can be built without changing the locale setting.
> 
> I agree with your reasoning, but I can not comment on the contents of the patch, because I'm not a maintainer of this code.
> 
> [1] : https://bugs.openjdk.java.net/browse/JDK-8268719

> Thanks for your suggestions, @JornVernee .
> I tested the above patch, but it still failed. (rm build -rf; bash configure ..; make images)

Ok, that's unfortunate. Thanks for testing.

-------------

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