On lines 228 and 1069 you should not mix p2d and this. I would go with
both p2d.pointTypes and p2d.numTypes in both cases...
...jim
On 3/6/15 2:03 PM, Laurent Bourgès wrote:
Phil,
Here is a new webrev:
http://cr.openjdk.java.net/~lbourges/webrev_Path2D_1/
See my comments below:
you placed the test in the java.awt.geom package.
25 package java.awt.geom;
and are accessing internals of that package.
In jigsaw/modular mode that won't even compile.
Ok it is annoying:
as all Path2D fields are package protected, I designed the test
using direct access to any fields ...
So the test should go in the anonymous package and avoid
accessing internals.
It should be possible to use just public API to verify the
arrays of a shape
being cloned are trimmed .
No, it is not possible to use Shape API to access arrays nor
fields (numTypes ...):
only getPathIterator() could give me data but it won't tell me if
the underlying arrays or fields are correct.
That is true ..
Well, if you need it to be in java.awt.geom, I think even today
you'll find it won't work
unless you jump through some jtreg hoops to install it on the
bootclasspath.
I think its something like "@run main/othervm -Xbootclasspath/a:. "
And later in the modular JDK it will need to be modified again.
I'd say either update the test to work with jtreg today - and test
it to be sure,
or provide the test without an @test tag, or with an @ignore tag, so
people can
still manually verify it but the harness won't run it.
I removed the @test tag but added comments indicating to run the test
manually.
Maybe I could use introspection to getDeclaredField(name) and
setAccessible(true) to get internal data.
That won't work either. So maybe this is a "noreg-hard" or
"noreg-cleanup" bug.
We add those labels to the JBS/JIRA bug when something isn't testable.
Nevermind.
Any idea or utility class I could use
Why is it necessary to explicitly add the call to super(); ?
223 super();
I agree it is not necessary but it explicitely says that I use the
empty constructor:
/**
* Constructs a new empty {@code Path2D} object.
* It is assumed that the package sibling subclass that is
* defaulting to this constructor will fill in all values.
*
* @since 1.6
*/
/* private protected */
Path2D() {
}
If we all did this, all of the time, there'd be a lot of extra lines
in the code, that the compiler
would fill in for us anyway.
I removed the superfluous super() calls.
Thanks for your review,
Laurent
