RFR: 8216154: C4819 warnings at HotSpot sources on Windows
Leo Korinth
leo.korinth at oracle.com
Mon Jan 7 14:32:06 UTC 2019
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
>
>
More information about the hotspot-compiler-dev
mailing list