RFR: 8216154: C4819 warnings at HotSpot sources on Windows

[Thomas Schatzl]

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