Alan, Paul,
Sometimes with these changes you do not want to cloud the significant
body of the change with other refactoring. That said, we are on round
two, so it seems reasonable.
I think I have captured/addressed all comments:
http://cr.openjdk.java.net/~chegar/8061297/webrev.01/webrev/
-Chris.
On 14/01/15 14:32, Paul Sandoz wrote:
On Jan 14, 2015, at 2:55 PM, Chris Hegarty <chris.hega...@oracle.com> wrote:
This test currently completes successfully, but indicates that it has "Parsed 0
classfiles". It doesn't do anything useful.
The modular image no longer contains jar files. The test should use the JRT
FileSystem to iterate over the image classfiles.
http://cr.openjdk.java.net/~chegar/8061297/webrev.00/webrev/
191 static class PlatformClassPath {
192 static List<Path> getClasses() throws IOException {
193 List<Path> result = new ArrayList<Path>();
194 Path home = Paths.get(System.getProperty("java.home"));
195
196 if (home.resolve("lib").toFile().exists()) {
197 // either an exploded build or an image
198 File classes = home.resolve("modules").toFile();
199 if (classes.exists() && classes.isDirectory()) {
200 result.add(classes.toPath());
201 } else {
202 JrtClasses jrt = new JrtClasses();
203 result.addAll(jrt.paths());
204 }
There is no need to copy the list. At line 200 one can return a singleton list.
The list is copied yet again if there are no explicit classes added:
73 if (classes.isEmpty()) {
74 classes.addAll(PlatformClassPath.getClasses());
75 }
You should be able to just copy the reference if classes is changed to List:
classes = PlatformClassPath.getClasses();
(IIUC there is just one pass over the classes so you could probably modify to
be totally lazy and process on the Stream directly but that would require
slightly more modification to the run method)
230 private Stream<Path> toStream(Path root) {
231 try {
232 return Files.walk(root);
233 } catch(IOException x) {
234 x.printStackTrace(System.err);
235 }
236 return Collections.<Path>emptyList().stream();
Is there a reason why the exception is swallowed? If not it would be better to
wrap in an UncheckedIOException and re-throw.
In general is this test thread safe if multiple tasks are submitted in a thread
pool? i.e. can the visit method be called concurrently and update the
non-synchronized csMethodsMissingAnnotation?
Paul.
