Hi Kim,
Thanks for looking at this!
Den 10/2/16 kl. 23:34, skrev Kim Barrett:
Den 10/2/16 kl. 21:31, skrev Jesper Wilhelmsson:
https://bugs.openjdk.java.net/browse/JDK-8149594
http://cr.openjdk.java.net/~jwilhelm/8149594/webrev.00/
------------------------------------------------------------------------------
I might have preferred two webrevs, one of only whitespace changes and
one of other changes.
Yes, I'll split it up in the next version if there is a need for it.
------------------------------------------------------------------------------
make/bsd/Makefile
make/linux/Makefile
172 TARGETS = debug fastdebug optimized product
...
200 BUILDTREE = $(MAKE) -f $(BUILDTREE_MAKE) $(BUILDTREE_VARS)
If there's a non-whitespace change (increasing the separation between
the variables and the "=" or "+="), I couldn't find it. I'm guessing
this is being done because the planned later changes introduce
something in here that leads to that reformatting?
Yes, this is one of those changes that is motivated by future changes. I wanted
to separate this whitespace change from the actual change to make that one
easier to review.
------------------------------------------------------------------------------
make/bsd/makefiles/gcc.make
278 WARNING_FLAGS += -Wconversion
Oh, cool! So we haven't been using that option after all!
Note: This is a "real change" that wasn't mentioned in the RFR.
I've been meaning to file a bug report against this for a while. The
pre-gcc4.3 version of -Wconversion probably ought not be used in a
production context anyway.
https://gcc.gnu.org/wiki/NewWconversion
The old behavior for -Wconversion was intended to aid translation of
old C code to modern C standards by identifying places where adding
function prototypes may result in different behavior. That's just not
an issue for C++, nor for our code in general.
And we're not prepared to use the new -Wconversion; see JDK-8135181.
So rather than changing our builds to actually use this option with
old compilers that Oracle doesn't support (so we can't locally test
this change), I suggest removing the option entirely, since it hasn't
actually been used anyway.
This typo was only present on bsd. Are you suggesting to remove it only on bsd,
or on linux as well?
------------------------------------------------------------------------------
make/bsd/makefiles/jvmti.make
make/linux/makefiles/trace.make
The only non-copyright change in these files seem to be the addition
of a blank line to the end of the file.
Yes, this is to make the bsd and linux versions of the files the same. It makes
it easier to apply patches from one platform to the other when porting stuff.
------------------------------------------------------------------------------
make/bsd/makefiles/top.make
88 vm_build_preliminaries: checks $(Cached_plat) $(AD_Files_If_Required)
trace_stuff jvmti_stuff dtrace_stuff
What is the point of re-ordering trace_stuff and jvmti_stuff?
To make the bsd and linux versions the same.
Also, elsewhere the whitespace after the target's ":" is minimized,
but not here.
Oops. I'll fix that.
/Jesper
------------------------------------------------------------------------------
