Hi Eric, On Wed, Nov 4, 2015 at 11:46 AM, Erik Helin <erik.he...@oracle.com> wrote:
> (adding build-dev as well) > > On 2015-11-03, Volker Simonis wrote: > > Hi Erik, > > > > while I agree that this is a nice clean up I want to second Thomas in > > his attempt to save the VS 2010 build. I understand that the official > > Oracle build tool chain for jdk9 on Windows is VS 2013, but this may > > be different for others. I also think that we shouldn't sacrifice the > > possibility to use VS 2010 without any hard requirements like for > > example the need to use new language features which are not supported > > in the old version. > > Hi Volker, > > I understand that you guys want to keep building VS2010 but I know that > you also understand that we want to get rid of this cruft from the GC > code. > > > If you and others still insist in doing this change, I'd strongly vote > > to first completely disable the support for VS 2010 in the 'configure' > > process before we start to do source code changes. > > > > Instead, I'd prefer if you could just change the code from: > > > > #ifdef _MSC_VER > > > > to: > > > > #if _MSC_VER < 1800 > > > > like it was already done for > > https://bugs.openjdk.java.net/browse/JDK-8137329. This makes the > > dependency on a specific compiler version explicit (although it would > > hardly count as a "clean up" any more :) > > Maybe we can come up with something that removes these #pragmas from the > GC code while still allowing you to compile with VS2010. What do you > think about the following suggestions: > - We change the Makefiles to add the flag /Wd4355 if we use VS2010 or > older. This will results in the warning being disabled for all files > (one could potentially also disable the warning per file). > - We add the code: > #if _MSC_VER < 1000 > #pragma warning( disable:4355 ) > #endif // _MSC_VER < 1800 > to globalDefinitions_visCPP.hpp. This file gets included in > globalDefinitions.hpp which in turn gets included basically > everywhere. This will turn off the warning for essentially all files. > > I'm leaning more towards the Makefile solution since that is where it > makes most sense to me to disable warnings. OTOH, > globalDefinitions_visCPP.hpp already disables quite a few warnings... > > Thanks! Both solutions are fine, I would prefer adding them to globalDefinitions_visCPP.hpp, which already contains a number of other workarounds for older MSVC versions. Kind Regards, Thomas > build-dev, I don't know if you guys have any other suggestion? > > Thanks, > Erik > > > Regards, > > Volker > > > > > > > > > > On Tue, Nov 3, 2015 at 3:32 PM, Thomas Stüfe <thomas.stu...@gmail.com> > wrote: > > > Hi, > > > > > > one question, does this mean this would not build anymore with Visual > Studio > > > 2010? > > > > > > According to : > > > https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms > , > > > VS2010 is still supported build platform. > > > > > > Also there seems to be enough interest in the community about keeping > the > > > VS2010 build alive, see > > > > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-October/015978.html > > > > > > We (SAP) unfortunately still use Visual Studio 2010 for our VM, it > would be > > > nice if that would continue to work. > > > > > > Best Regards, Thomas > > > > > > > > > > > > On Mon, Nov 2, 2015 at 5:17 PM, Erik Helin <erik.he...@oracle.com> > wrote: > > >> > > >> On 2015-11-02, Thomas Schatzl wrote: > > >> > Hi all, > > >> > > > >> > Erik H. made me aware of another pragma 4355 in nmethod.cpp that > can > > >> > be removed. > > >> > > > >> > I updated the webrev in place because of the kind of change. > > >> > > >> Looks good, Reviewed. > > >> > > >> > Thanks, > > >> > Thomas > > >> > > > >> > On Mon, 2015-11-02 at 16:14 +0100, Thomas Schatzl wrote: > > >> > > Hi all, > > >> > > > > >> > > can I have reviews for the following change that removes some > > >> > > pragmas > > >> > > in GC code? > > >> > > With the move to newer MSVC, they are not needed any more. > > >> > > > > >> > > CR: > > >> > > https://bugs.openjdk.java.net/browse/JDK-8141134 > > >> > > Webrev: > > >> > > http://cr.openjdk.java.net/~tschatzl/8141134/webrev/ > > >> > > Testing: > > >> > > jprt > > >> > > > > >> > > Thanks, > > >> > > Thomas > > >> > > > > >> > > > > >> > > > >> > > > > > > > >