Skip to content

Commit dbb562d

Browse files
lahodajlahodaj
committed
8303355: The Depend plugin does fully recompile when primitive type changes
Reviewed-by: erikj, vromero
1 parent 4619e8b commit dbb562d

File tree

2 files changed

+90
-36
lines changed

2 files changed

+90
-36
lines changed

‎make/jdk/src/classes/build/tools/depend/Depend.java

+41-33
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.sun.source.tree.MemberSelectTree;
3434
import com.sun.source.tree.MethodTree;
3535
import com.sun.source.tree.ModifiersTree;
36+
import com.sun.source.tree.PrimitiveTypeTree;
3637
import com.sun.source.tree.Tree;
3738
import com.sun.source.tree.VariableTree;
3839
import java.io.File;
@@ -140,9 +141,12 @@ public String getName() {
140141
public void init(JavacTask jt, String... args) {
141142
addExports();
142143

144+
Path internalAPIDigestFile;
145+
Map<String, String> internalAPI = new HashMap<>();
143146
AtomicBoolean noApiChange = new AtomicBoolean();
147+
Context context = ((BasicJavacTask) jt).getContext();
148+
JavaCompiler compiler = JavaCompiler.instance(context);
144149
try {
145-
Context context = ((BasicJavacTask) jt).getContext();
146150
Options options = Options.instance(context);
147151
String modifiedInputs = options.get("modifiedInputs");
148152
if (modifiedInputs == null) {
@@ -157,8 +161,18 @@ public void init(JavacTask jt, String... args) {
157161
Set<Path> modified = Files.readAllLines(Paths.get(modifiedInputs)).stream()
158162
.map(Paths::get)
159163
.collect(Collectors.toSet());
160-
Path internalAPIDigestFile = Paths.get(internalAPIPath);
161-
JavaCompiler compiler = JavaCompiler.instance(context);
164+
internalAPIDigestFile = Paths.get(internalAPIPath);
165+
if (Files.isReadable(internalAPIDigestFile)) {
166+
try {
167+
Files.readAllLines(internalAPIDigestFile, StandardCharsets.UTF_8)
168+
.forEach(line -> {
169+
String[] keyAndValue = line.split("=");
170+
internalAPI.put(keyAndValue[0], keyAndValue[1]);
171+
});
172+
} catch (IOException ex) {
173+
throw new IllegalStateException(ex);
174+
}
175+
}
162176
Class<?> initialFileParserIntf = Class.forName("com.sun.tools.javac.main.JavaCompiler$InitialFileParserIntf");
163177
Class<?> initialFileParser = Class.forName("com.sun.tools.javac.main.JavaCompiler$InitialFileParser");
164178
Field initialParserKeyField = initialFileParser.getDeclaredField("initialParserKey");
@@ -169,7 +183,7 @@ public void init(JavacTask jt, String... args) {
169183
new Class<?>[] {initialFileParserIntf},
170184
new FilteredInitialFileParser(compiler,
171185
modified,
172-
internalAPIDigestFile,
186+
internalAPI,
173187
noApiChange,
174188
debug));
175189
context.<Object>put(key, initialParserInstance);
@@ -213,7 +227,17 @@ public void finished(TaskEvent te) {
213227
}
214228
}
215229
}
216-
if (te.getKind() == Kind.COMPILATION && !noApiChange.get()) {
230+
if (te.getKind() == Kind.COMPILATION && !noApiChange.get() &&
231+
compiler.errorCount() == 0) {
232+
try (OutputStream out = Files.newOutputStream(internalAPIDigestFile)) {
233+
String hashes = internalAPI.entrySet()
234+
.stream()
235+
.map(e -> e.getKey() + "=" + e.getValue())
236+
.collect(Collectors.joining("\n"));
237+
out.write(hashes.getBytes(StandardCharsets.UTF_8));
238+
} catch (IOException ex) {
239+
throw new IllegalStateException(ex);
240+
}
217241
String previousSignature = null;
218242
File digestFile = new File(args[0]);
219243
try (InputStream in = new FileInputStream(digestFile)) {
@@ -258,20 +282,8 @@ public ClassLoader getClassLoader(JavaFileManager.Location location) {
258282

259283
private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
260284
JavaCompiler compiler, Iterable<JavaFileObject> fileObjects, Set<Path> modified,
261-
Path internalAPIDigestFile, AtomicBoolean noApiChange,
285+
Map<String, String> internalAPI, AtomicBoolean noApiChange,
262286
boolean debug) {
263-
Map<String, String> internalAPI = new LinkedHashMap<>();
264-
if (Files.isReadable(internalAPIDigestFile)) {
265-
try {
266-
Files.readAllLines(internalAPIDigestFile, StandardCharsets.UTF_8)
267-
.forEach(line -> {
268-
String[] keyAndValue = line.split("=");
269-
internalAPI.put(keyAndValue[0], keyAndValue[1]);
270-
});
271-
} catch (IOException ex) {
272-
throw new IllegalStateException(ex);
273-
}
274-
}
275287
Map<JavaFileObject, JCCompilationUnit> files2CUT = new IdentityHashMap<>();
276288
boolean fullRecompile = modified.stream()
277289
.map(Path::toString)
@@ -289,7 +301,6 @@ private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
289301
result.add(parsed);
290302
}
291303
}
292-
293304
if (fullRecompile) {
294305
for (JavaFileObject jfo : fileObjects) {
295306
if (!modified.contains(Path.of(jfo.getName()))) {
@@ -301,15 +312,6 @@ private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
301312
result.add(parsed);
302313
}
303314
}
304-
try (OutputStream out = Files.newOutputStream(internalAPIDigestFile)) {
305-
String hashes = internalAPI.entrySet()
306-
.stream()
307-
.map(e -> e.getKey() + "=" + e.getValue())
308-
.collect(Collectors.joining("\n"));
309-
out.write(hashes.getBytes(StandardCharsets.UTF_8));
310-
} catch (IOException ex) {
311-
throw new IllegalStateException(ex);
312-
}
313315
} else {
314316
noApiChange.set(true);
315317
}
@@ -835,7 +837,7 @@ private boolean importantMember(Tree m) {
835837
!isPrivate(((VariableTree) m).getModifiers()) ||
836838
isRecordComponent((VariableTree) m);
837839
case BLOCK -> false;
838-
default -> throw new IllegalStateException("Unexpected tree kind: " + m.getKind());
840+
default -> false;
839841
};
840842
}
841843

@@ -878,24 +880,30 @@ public Void visitModifiers(ModifiersTree node, Void p) {
878880
return super.visitModifiers(node, p);
879881
}
880882

883+
@Override
884+
public Void visitPrimitiveType(PrimitiveTypeTree node, Void p) {
885+
update(node.getPrimitiveTypeKind().name());
886+
return super.visitPrimitiveType(node, p);
887+
}
888+
881889
}
882890

883891
private class FilteredInitialFileParser implements InvocationHandler {
884892

885893
private final JavaCompiler compiler;
886894
private final Set<Path> modified;
887-
private final Path internalAPIDigestFile;
895+
private final Map<String, String> internalAPI;
888896
private final AtomicBoolean noApiChange;
889897
private final boolean debug;
890898

891899
public FilteredInitialFileParser(JavaCompiler compiler,
892900
Set<Path> modified,
893-
Path internalAPIDigestFile,
901+
Map<String, String> internalAPI,
894902
AtomicBoolean noApiChange,
895903
boolean debug) {
896904
this.compiler = compiler;
897905
this.modified = modified;
898-
this.internalAPIDigestFile = internalAPIDigestFile;
906+
this.internalAPI = internalAPI;
899907
this.noApiChange = noApiChange;
900908
this.debug = debug;
901909
}
@@ -907,7 +915,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
907915
case "parse" -> doFilteredParse(compiler,
908916
(Iterable<JavaFileObject>) args[0],
909917
modified,
910-
internalAPIDigestFile,
918+
internalAPI,
911919
noApiChange,
912920
debug);
913921
default -> throw new UnsupportedOperationException();

‎make/jdk/src/classes/build/tools/depend/DependTest.java

+49-3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.io.File;
2929
import java.io.IOException;
3030
import java.io.OutputStream;
31+
import java.io.Writer;
3132
import java.net.URI;
3233
import java.net.URISyntaxException;
3334
import java.nio.file.Files;
@@ -54,6 +55,8 @@ public static void main(String... args) throws Exception {
5455
test.testRecords();
5556
test.testImports();
5657
test.testModifiers();
58+
test.testPrimitiveTypeChanges();
59+
test.testWithErrors();
5760
}
5861

5962
public void testMethods() throws Exception {
@@ -302,6 +305,27 @@ public void testRecords() throws Exception {
302305
"package test; public record Test (int x, int y) {" +
303306
"public Test (int x, int y, int z) { this(x, y); } }", // additional ctr
304307
true);
308+
doOrdinaryTest("package test; public record Test (int x) { }",
309+
"package test; public record Test (long x) { unresolved f; }", //erroneous record member, should not crash
310+
false);
311+
}
312+
313+
public void testPrimitiveTypeChanges() throws Exception {
314+
doOrdinaryTest("package test; public record Test (int x) { }",
315+
"package test; public record Test (long x) { }",
316+
true);
317+
doOrdinaryTest("package test; public record Test (int x) { }",
318+
"package test; public record Test (Integer x) { }",
319+
true);
320+
doOrdinaryTest("package test; public record Test (Integer x) { }",
321+
"package test; public record Test (int x) { }",
322+
true);
323+
}
324+
325+
public void testWithErrors() throws Exception {
326+
doOrdinaryTest("package test; public record Test (int x) { }",
327+
"package test; public record Test (long x) { static unresolved f; }",
328+
false); //the API change should not be recorded for code with errors
305329
}
306330

307331
private final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
@@ -310,6 +334,7 @@ public void testRecords() throws Exception {
310334
private Path scratchClasses;
311335
private Path apiHash;
312336
private Path treeHash;
337+
private Path modifiedFiles;
313338

314339
private void setupClass() throws IOException {
315340
depend = Paths.get(Depend.class.getProtectionDomain().getCodeSource().getLocation().getPath());
@@ -328,18 +353,24 @@ private void setupClass() throws IOException {
328353

329354
apiHash = scratch.resolve("api");
330355
treeHash = scratch.resolve("tree");
356+
modifiedFiles = scratch.resolve("modified-files");
331357
}
332358

333359
private void doOrdinaryTest(String codeBefore, String codeAfter, boolean hashChangeExpected) throws Exception {
334360
doOrdinaryTest(codeBefore, codeAfter, hashChangeExpected, hashChangeExpected);
335361
}
336362

337363
private void doOrdinaryTest(String codeBefore, String codeAfter, boolean apiHashChangeExpected, boolean treeHashChangeExpected) throws Exception {
364+
try (Writer out = Files.newBufferedWriter(modifiedFiles)) {
365+
out.append("module-info.java\n");
366+
out.append("test.Test.java\n");
367+
}
338368
List<String> options =
339369
Arrays.asList("-d", scratchClasses.toString(),
340370
"-processorpath", depend.toString() + File.pathSeparator + scratchServices.toString(),
341-
"-Xplugin:depend " + apiHash.toString() + " " + treeHash.toString(),
342-
"-XDmodifiedInputs=build-all");
371+
"-Xplugin:depend " + apiHash.toString(),
372+
"-XDinternalAPIPath=" + treeHash.toString(),
373+
"-XDmodifiedInputs=" + modifiedFiles.toString());
343374
List<TestJavaFileObject> beforeFiles =
344375
Arrays.asList(new TestJavaFileObject("module-info", "module m { exports test; }"),
345376
new TestJavaFileObject("test.Test", codeBefore));
@@ -369,11 +400,19 @@ private void doModuleTest(String codeBefore, String codeAfter, boolean hashChang
369400
}
370401

371402
private void doModuleTest(String codeBefore, String codeAfter, boolean apiHashChangeExpected, boolean treeHashChangeExpected) throws Exception {
403+
try (Writer out = Files.newBufferedWriter(modifiedFiles)) {
404+
out.append("module-info.java\n");
405+
out.append("test.Test1.java\n");
406+
out.append("test.Test2.java\n");
407+
out.append("test.TestImpl1.java\n");
408+
out.append("test.TestImpl2.java\n");
409+
}
372410
List<String> options =
373411
Arrays.asList("-d", scratchClasses.toString(),
374412
"-processorpath", depend.toString() + File.pathSeparator + scratchServices.toString(),
375413
"-Xplugin:depend " + apiHash.toString() + " " + treeHash.toString(),
376-
"-XDmodifiedInputs=build-all");
414+
"-XDinternalAPIPath=" + treeHash.toString(),
415+
"-XDmodifiedInputs=" + modifiedFiles.toString());
377416
List<TestJavaFileObject> beforeFiles =
378417
Arrays.asList(new TestJavaFileObject("module-info", codeBefore),
379418
new TestJavaFileObject("test.Test1", "package test; public interface Test1 {}"),
@@ -406,10 +445,12 @@ private void doModuleTest(String codeBefore, String codeAfter, boolean apiHashCh
406445

407446
private static final class TestJavaFileObject extends SimpleJavaFileObject {
408447

448+
private final String className;
409449
private final String code;
410450

411451
public TestJavaFileObject(String className, String code) throws URISyntaxException {
412452
super(new URI("mem:/" + className.replace('.', '/') + ".java"), Kind.SOURCE);
453+
this.className = className;
413454
this.code = code;
414455
}
415456

@@ -418,5 +459,10 @@ public CharSequence getCharContent(boolean arg0) throws IOException {
418459
return code;
419460
}
420461

462+
@Override
463+
public String getName() {
464+
return className + ".java";
465+
}
466+
421467
}
422468
}

0 commit comments

Comments
 (0)