> On Jun 30, 2020, at 6:48 AM, Magnus Ihse Bursie 
> <magnus.ihse.bur...@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.bur...@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.

Reply via email to