RFR: 8216154: C4819 warnings at HotSpot sources on Windows

[Yasumasa Suenaga]

Hi Thomas,

On 2019/01/07 22: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?

Sorry, it's my mistake.
But the error will occur in its file because `RANGE0` which contains non-ASCII
characters passes to sscanf().


> 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.

Ok, I will add the comment.


Thanks,

Yasumasa


> Thanks,
>    Thomas
> 
>