Quick reply from my vacation... there's a macro that tests if a compiler option is valid. We've only used it for gcc so far, but I see no reason for why it should not work with VS. Maybe you can use that instead of checking version?
I'm a bit hesitant at adding options that will produce warnings for older but still supported versions. If it's just a single compiler at the end of the list, that's one thing, but if we're talking everything but VS2017, that's another. If there's a problem adding the VS option, I'd rather see you just add the gcc one. /Magnus > 7 aug. 2018 kl. 12:08 skrev David Holmes <david.hol...@oracle.com>: > > Hi Thomas, > >> On 7/08/2018 7:34 PM, Thomas Schatzl wrote: >> Hi David, >>> On Tue, 2018-08-07 at 07:17 +1000, David Holmes wrote: >>> Hi Thomas, >>> >>>> On 6/08/2018 10:38 PM, Thomas Schatzl wrote: >>>> Hi David, >>>> >>>>> On Fri, 2018-08-03 at 10:20 +1000, David Holmes wrote: >>>>> Hi Thomas, >>>>> >>>>>> On 2/08/2018 7:14 PM, Thomas Schatzl wrote: >>>>>> Hi all, >>>>>> >>>>>> there have been several suggestions to try to fix the >>>>>> Hotspot code to allow us to enable -Wreorder in the Hotspot >>>>>> sources. >>>>>> This should make problems due to use-before-initialization much >>>>>> more obvious. >>>>>> >>>>>> This change enables -Wreorder for gcc and clang. >>>>> >>>>> For Windows (VS2017+) see: >>>>> >>>>> https://docs.microsoft.com/en-us/cpp/error-messages/compiler- >>>>> warnings /c5038 >>>>> >>>>> Otherwise change seems okay. >>>>> >>>> >>>> Thanks! >>>> >>>> http://cr.openjdk.java.net/~tschatzl/8208672/webrev.0_to_1 (diff) >>>> http://cr.openjdk.java.net/~tschatzl/8208672/webrev.1 (full) >>>> >>>> I verified that all our platforms (including Windows) still build. >>> >>> I think the Windows change needs to be based on the compiler version >>> used as, from what I read, the flag only exists in VS2017. >> jdk11 official compiler is VS2017 so I figured it would not be that >> much of a problem. > > Right now the build supports a range of compilers for the community: > > ################################################################################ > # The order of these defines the priority by which we try to find them. > VALID_VS_VERSIONS="2017 2013 2015 2012 2010" > >> Another reason for my thinking is that MSVC 2013 does not implement >> some interesting C++11 features anyway so we might be forced to drop >> support for it soon (and the situation is of course worse if we upgrade >> to C++14). > > If that happens we'd have to deal with it sure ... > >> I looked a bit at the makefile and conditionalizing this on VS2017 (or >> checking only whether VS2013 let it slide, i.e. give an "unsupported >> option" warning but continue anyway because I would need to setup a >> Windows dev environment somewhere) would take me a lot of time. > > Hoping the build folk will jump in as this should be pretty straight-forward > I think - just a question of knowing what VERSION variable to test. > >> Would it be possible to skip -Werror support for Visual Studio now and >> try to fix this in a later enhancement? > > Sure. Just a pity as it will be 10x as much work to add later by itself. > > Thanks, > David > ----- > >> Thanks, >> Thomas
