RFR: 8216154: C4819 warnings at HotSpot sources on Windows

[Leo Korinth]

Hi!

On 07/01/2019 15:53, Yasumasa Suenaga wrote:
> Hi Leo,
> 
>> 1) Should not all of those files be fixed?
> 
> 
>> ./src/hotspot/cpu/x86/macroAssembler_x86_sha.cpp:                          
>> C source, UTF-8 Unicode text
>> ./src/hotspot/share/oops/method.cpp:                                                                   
>> C source, UTF-8 Unicode text
> 
> Non-ASCII character(s) in comment line.
> 
> 
>> ./src/hotspot/cpu/aarch64/macroAssembler_aarch64_trig.cpp:                          
>> C source, UTF-8 Unicode text
> 
> It's not used for Windows (AArch64).
> 
> 
>> ./src/hotspot/share/gc/parallel/gcTaskManager.hpp:                          
>> data
> 
> I couldn't find why `file` detects it as "data". So I have no idea for it.
> 
> 
>> ./src/hotspot/share/code/codeHeapState.cpp:                          C 
>> source, UTF-8 Unicode text
> 
> It's ASCII file on my laptop :-)


Check line 1979:

00015f20: 7468 6579 2068 6176 6520 6e6f 2063 6f6d  they have no com
00015f30: 7069 6c61 7469 6f6e c2a0 4944 2061 7373  pilation..ID ass

c2a0 is not ASCII but Latin-1 Supplement (NO-BREAK SPACE) and is not in 
a comment.

I do not have a windows environment up and running. The compiler seems 
to warn in strange places, and _not_ warn in others. I can not get a 
confirmation in the style guide that we are to use only ASCII, so please 
ignore my questions as I do not know what to recommend.

Thanks,
Leo

> 
> 
>> ./test/hotspot/gtest/utilities/test_json.cpp:                                                                   
>> C source, UTF-8 Unicode text
> 
> It's test code.
> 
> 
>> 2) Why remove warning (in one file, methodMatcher.cpp) instead of 
>> changing encoding?
>> 3) methodMatcher.cpp seems to be pure ASCII, why the change in that 
>> file at all?
> 
> The error occurs about `RANGE0`. It has binary data, so it might not be 
> able to change encoding. >
> Thanks,
> 
> Yasumasa
> 
> 
> On 2019/01/07 23:32, Leo Korinth wrote:
>> Hi!
>>
>> Running: find -name "*.[ch]pp" | xargs file | grep -v ASCII
>> ./src/hotspot/cpu/x86/macroAssembler_x86_sha.cpp: 
>>                          C source, UTF-8 Unicode text
>> ./src/hotspot/cpu/aarch64/macroAssembler_aarch64_trig.cpp: 
>>                          C source, UTF-8 Unicode text
>> ./src/hotspot/share/gc/parallel/gcTaskManager.hpp: 
>>                          data
>> ./src/hotspot/share/code/codeHeapState.cpp:                          C 
>> source, UTF-8 Unicode text
>> ./src/hotspot/share/oops/method.cpp: 
>>                                                                   C 
>> source, UTF-8 Unicode text
>> ./test/hotspot/gtest/utilities/test_json.cpp: 
>>                                                                   C 
>> source, UTF-8 Unicode text
>>
>>
>> The single hpp file seems fine though (just file not understanding 
>> that it is a source file).
>>
>> Some questions, as it seems like I am missing something.
>> 1) Should not all of those files be fixed?
>> 2) Why remove warning (in one file, methodMatcher.cpp) instead of 
>> changing encoding?
>> 3) methodMatcher.cpp seems to be pure ASCII, why the change in that 
>> file at all?
>>
>> $ grep --color -P -n "[^[:ascii:]]" is a good way to find the 
>> problematic line.
>>
>> Thanks, Leo
>>
>> On 07/01/2019 14:20, Thomas Schatzl wrote:
>>> Hi,
>>>
>>> On Mon, 2019-01-07 at 21:36 +0900, Yasumasa Suenaga wrote:
>>>> Hi Kim,
>>>>
>>>> On 2019/01/07 7:18, Kim Barrett wrote:
>>>>>> On Jan 6, 2019, at 12:54 PM, Kim Barrett <kim.barrett at oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>>> On Jan 6, 2019, at 7:53 AM, Yasumasa Suenaga <
>>>>>>> yasuenag at gmail.com> wrote:
>>>>>>>
>>>>>>> Hi Kim,
>>>>>>>
>>>>>>> Thank you for your comment.
>>>>>>> I uploaded new webrev to use pragma warning push/pop:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8216154/webrev.01/
>>>>>>>
>>>>>>>
>>>>>>> Please review again.
>>>>>>
>>>>>> Looks good.
>>>
>>> I tried to verify these problems on these two files as suggested with
>>> "iconv -f US-ASCII -t UTF8 <file>" which errored out on
>>> codeHeapState.cpp as expected but there has been no error with
>>> methodMatcher.cpp. Am I doing something wrong?
>>>
>>> I am fine with that change if it is really needed for successful
>>> compliation :) I just can't find the non-US-ASCII character used in the
>>> line indicated by the error message.
>>>
>>>>>
>>>>> It later occurred to me to wonder whether _WINDOWS was the right
>>>>> macro to conditionalize on.  All other uses of #pragma warning
>>>>> push/pop (there are 5 in HotSpot) use _MSC_VER.
>>>>
>>>> I updated webrev to use _MSC_VER. Is it ok?
>>>>
>>>>     http://cr.openjdk.java.net/~ysuenaga/JDK-8216154/webrev.02/
>>>>
>>>
>>> Please add a "// warning C4189: The file contains a character that
>>> cannot be represented in the current code page" comment above or next
>>> to the pragma warning(disable) declaration.
>>>
>>> Not many people know the VC warning numbers by default...
>>>
>>> Looks good otherwise, I do not need a re-review for this comment
>>> change.
>>>
>>> Thanks,
>>>    Thomas
>>>
>>>