RFR: JDK-8248548 Use DISABLED_WARNINGS for globally disabled warnings on Visual Studio in Hotspot
Kim Barrett
kim.barrett at oracle.com
Tue Jun 30 10:54:58 UTC 2020
> On Jun 30, 2020, at 6:48 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
>
> On 2020-06-30 12:32, Kim Barrett wrote:
>>> On Jun 30, 2020, at 6:08 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
>>>
>>> Currently hotspot/share/utilities/globalDefinitions_visCPP.hpp contains a lot of #pragma warning( disable : ...).
>>>
>>> All these globally disabled warnings should move to the make files instead.
>>>
>>> I also cleaned out some versions checks that are no longer relevant for the range of supported versions of Microsoft Visual Studio.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8248548
>>> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8248548-use-DISABLED_WARNINGS_microsoft-for-hotspot/webrev.01
>>>
>>> /Magnus
>> I think it would be nice to have a comment block describing each of the disabled warnings.
>> But thanks for sorting them, unlike the globalDefinitions code.
>>
>> (Even better would be to have comments describing why we’re disabling them, but we don’t do
>> that for any other platform either.)
>>
> We used to have that for solaris. It ended up as a long document inside the code, hiding the actual code. I agree that it would be good to have a rationale on why we disable warnings, and that it should be documented. But I think the source code is the wrong location for that. Sure, non-descriptive warnings designations like microsoft has is typically more in need of documentation than the gcc-style, but the important thing is *why* it is disabled, not *what* is disabled.
>
> So I'd argue that we should not pollute the source code with lines upon lines of warning messages, but if anything, we should instead point to an external location where we can provide not only an explanation of what the warnings does, but a rationale for disabling it.
>
> /Magnus
I can understand that. Maybe a job for the HotSpot Style Guide. Oh wait, what am I saying.
Change looks good.
More information about the build-dev
mailing list