stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8303355: The Depend plugin does fully recompile when primitive type changes

Browse Source 
Reviewed-by: erikj, vromero
This commit is contained in:
Jan Lahoda 2023-03-02 09:41:11 +00:00
parent 4619e8bae8
commit dbb562d3b1
2 changed files with 90 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
make/jdk/src/classes/build/tools/depend/Depend.java
View File

@ -33,6 +33,7 @@ import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.ModifiersTree;
import com.sun.source.tree.PrimitiveTypeTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import java.io.File;
@ -140,9 +141,12 @@ public class Depend implements Plugin {
public void init(JavacTask jt, String... args) {
addExports();
Path internalAPIDigestFile;
Map<String, String> internalAPI = new HashMap<>();
AtomicBoolean noApiChange = new AtomicBoolean();
Context context = ((BasicJavacTask) jt).getContext();
JavaCompiler compiler = JavaCompiler.instance(context);
try {
Context context = ((BasicJavacTask) jt).getContext();
Options options = Options.instance(context);
String modifiedInputs = options.get("modifiedInputs");
if (modifiedInputs == null) {
@ -157,8 +161,18 @@ public class Depend implements Plugin {
Set<Path> modified = Files.readAllLines(Paths.get(modifiedInputs)).stream()
.map(Paths::get)
.collect(Collectors.toSet());
Path internalAPIDigestFile = Paths.get(internalAPIPath);
JavaCompiler compiler = JavaCompiler.instance(context);
internalAPIDigestFile = Paths.get(internalAPIPath);
if (Files.isReadable(internalAPIDigestFile)) {
try {
Files.readAllLines(internalAPIDigestFile, StandardCharsets.UTF_8)
.forEach(line -> {
String[] keyAndValue = line.split("=");
internalAPI.put(keyAndValue[0], keyAndValue[1]);
});
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
}
Class<?> initialFileParserIntf = Class.forName("com.sun.tools.javac.main.JavaCompiler$InitialFileParserIntf");
Class<?> initialFileParser = Class.forName("com.sun.tools.javac.main.JavaCompiler$InitialFileParser");
Field initialParserKeyField = initialFileParser.getDeclaredField("initialParserKey");
@ -169,7 +183,7 @@ public class Depend implements Plugin {
new Class<?>[] {initialFileParserIntf},
new FilteredInitialFileParser(compiler,
modified,
internalAPIDigestFile,
internalAPI,
noApiChange,
debug));
context.<Object>put(key, initialParserInstance);
@ -213,7 +227,17 @@ public class Depend implements Plugin {
}
}
}
if (te.getKind() == Kind.COMPILATION && !noApiChange.get()) {
if (te.getKind() == Kind.COMPILATION && !noApiChange.get() &&
compiler.errorCount() == 0) {
try (OutputStream out = Files.newOutputStream(internalAPIDigestFile)) {
String hashes = internalAPI.entrySet()
.stream()
.map(e -> e.getKey() + "=" + e.getValue())
.collect(Collectors.joining("\n"));
out.write(hashes.getBytes(StandardCharsets.UTF_8));
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
String previousSignature = null;
File digestFile = new File(args[0]);
try (InputStream in = new FileInputStream(digestFile)) {
@ -258,20 +282,8 @@ public class Depend implements Plugin {
private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
JavaCompiler compiler, Iterable<JavaFileObject> fileObjects, Set<Path> modified,
Path internalAPIDigestFile, AtomicBoolean noApiChange,
Map<String, String> internalAPI, AtomicBoolean noApiChange,
boolean debug) {
Map<String, String> internalAPI = new LinkedHashMap<>();
if (Files.isReadable(internalAPIDigestFile)) {
try {
Files.readAllLines(internalAPIDigestFile, StandardCharsets.UTF_8)
.forEach(line -> {
String[] keyAndValue = line.split("=");
internalAPI.put(keyAndValue[0], keyAndValue[1]);
});
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
}
Map<JavaFileObject, JCCompilationUnit> files2CUT = new IdentityHashMap<>();
boolean fullRecompile = modified.stream()
.map(Path::toString)
@ -289,7 +301,6 @@ public class Depend implements Plugin {
result.add(parsed);
}
}
if (fullRecompile) {
for (JavaFileObject jfo : fileObjects) {
if (!modified.contains(Path.of(jfo.getName()))) {
@ -301,15 +312,6 @@ public class Depend implements Plugin {
result.add(parsed);
}
}
try (OutputStream out = Files.newOutputStream(internalAPIDigestFile)) {
String hashes = internalAPI.entrySet()
.stream()
.map(e -> e.getKey() + "=" + e.getValue())
.collect(Collectors.joining("\n"));
out.write(hashes.getBytes(StandardCharsets.UTF_8));
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
} else {
noApiChange.set(true);
}
@ -835,7 +837,7 @@ public class Depend implements Plugin {
!isPrivate(((VariableTree) m).getModifiers()) ||
isRecordComponent((VariableTree) m);
case BLOCK -> false;
default -> throw new IllegalStateException("Unexpected tree kind: " + m.getKind());
default -> false;
};
}
@ -878,24 +880,30 @@ public class Depend implements Plugin {
return super.visitModifiers(node, p);
}
@Override
public Void visitPrimitiveType(PrimitiveTypeTree node, Void p) {
update(node.getPrimitiveTypeKind().name());
return super.visitPrimitiveType(node, p);
}
}
private class FilteredInitialFileParser implements InvocationHandler {
private final JavaCompiler compiler;
private final Set<Path> modified;
private final Path internalAPIDigestFile;
private final Map<String, String> internalAPI;
private final AtomicBoolean noApiChange;
private final boolean debug;
public FilteredInitialFileParser(JavaCompiler compiler,
Set<Path> modified,
Path internalAPIDigestFile,
Map<String, String> internalAPI,
AtomicBoolean noApiChange,
boolean debug) {
this.compiler = compiler;
this.modified = modified;
this.internalAPIDigestFile = internalAPIDigestFile;
this.internalAPI = internalAPI;
this.noApiChange = noApiChange;
this.debug = debug;
}
@ -907,7 +915,7 @@ public class Depend implements Plugin {
case "parse" -> doFilteredParse(compiler,
(Iterable<JavaFileObject>) args[0],
modified,
internalAPIDigestFile,
internalAPI,
noApiChange,
debug);
default -> throw new UnsupportedOperationException();

52
make/jdk/src/classes/build/tools/depend/DependTest.java
View File

@ -28,6 +28,7 @@ import com.sun.source.util.Plugin;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Files;
@ -54,6 +55,8 @@ public class DependTest {
test.testRecords();
test.testImports();
test.testModifiers();
test.testPrimitiveTypeChanges();
test.testWithErrors();
}
public void testMethods() throws Exception {
@ -302,6 +305,27 @@ public class DependTest {
"package test; public record Test (int x, int y) {" +
"public Test (int x, int y, int z) { this(x, y); } }", // additional ctr
true);
doOrdinaryTest("package test; public record Test (int x) { }",
"package test; public record Test (long x) { unresolved f; }", //erroneous record member, should not crash
false);
}
public void testPrimitiveTypeChanges() throws Exception {
doOrdinaryTest("package test; public record Test (int x) { }",
"package test; public record Test (long x) { }",
true);
doOrdinaryTest("package test; public record Test (int x) { }",
"package test; public record Test (Integer x) { }",
true);
doOrdinaryTest("package test; public record Test (Integer x) { }",
"package test; public record Test (int x) { }",
true);
}
public void testWithErrors() throws Exception {
doOrdinaryTest("package test; public record Test (int x) { }",
"package test; public record Test (long x) { static unresolved f; }",
false); //the API change should not be recorded for code with errors
}
private final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
@ -310,6 +334,7 @@ public class DependTest {
private Path scratchClasses;
private Path apiHash;
private Path treeHash;
private Path modifiedFiles;
private void setupClass() throws IOException {
depend = Paths.get(Depend.class.getProtectionDomain().getCodeSource().getLocation().getPath());
@ -328,6 +353,7 @@ public class DependTest {
apiHash = scratch.resolve("api");
treeHash = scratch.resolve("tree");
modifiedFiles = scratch.resolve("modified-files");
}
private void doOrdinaryTest(String codeBefore, String codeAfter, boolean hashChangeExpected) throws Exception {
@ -335,11 +361,16 @@ public class DependTest {
}
private void doOrdinaryTest(String codeBefore, String codeAfter, boolean apiHashChangeExpected, boolean treeHashChangeExpected) throws Exception {
try (Writer out = Files.newBufferedWriter(modifiedFiles)) {
out.append("module-info.java\n");
out.append("test.Test.java\n");
}
List<String> options =
Arrays.asList("-d", scratchClasses.toString(),
"-processorpath", depend.toString() + File.pathSeparator + scratchServices.toString(),
"-Xplugin:depend " + apiHash.toString() + " " + treeHash.toString(),
"-XDmodifiedInputs=build-all");
"-Xplugin:depend " + apiHash.toString(),
"-XDinternalAPIPath=" + treeHash.toString(),
"-XDmodifiedInputs=" + modifiedFiles.toString());
List<TestJavaFileObject> beforeFiles =
Arrays.asList(new TestJavaFileObject("module-info", "module m { exports test; }"),
new TestJavaFileObject("test.Test", codeBefore));
@ -369,11 +400,19 @@ public class DependTest {
}
private void doModuleTest(String codeBefore, String codeAfter, boolean apiHashChangeExpected, boolean treeHashChangeExpected) throws Exception {
try (Writer out = Files.newBufferedWriter(modifiedFiles)) {
out.append("module-info.java\n");
out.append("test.Test1.java\n");
out.append("test.Test2.java\n");
out.append("test.TestImpl1.java\n");
out.append("test.TestImpl2.java\n");
}
List<String> options =
Arrays.asList("-d", scratchClasses.toString(),
"-processorpath", depend.toString() + File.pathSeparator + scratchServices.toString(),
"-Xplugin:depend " + apiHash.toString() + " " + treeHash.toString(),
"-XDmodifiedInputs=build-all");
"-XDinternalAPIPath=" + treeHash.toString(),
"-XDmodifiedInputs=" + modifiedFiles.toString());
List<TestJavaFileObject> beforeFiles =
Arrays.asList(new TestJavaFileObject("module-info", codeBefore),
new TestJavaFileObject("test.Test1", "package test; public interface Test1 {}"),
@ -406,10 +445,12 @@ public class DependTest {
private static final class TestJavaFileObject extends SimpleJavaFileObject {
private final String className;
private final String code;
public TestJavaFileObject(String className, String code) throws URISyntaxException {
super(new URI("mem:/" + className.replace('.', '/') + ".java"), Kind.SOURCE);
this.className = className;
this.code = code;
}
@ -418,5 +459,10 @@ public class DependTest {
return code;
}
@Override
public String getName() {
return className + ".java";
}
}
}