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