From d428dc697aadf12221cdc83b3844c795385f3fe7 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Wed, 22 May 2024 13:56:32 -0400 Subject: [PATCH 1/6] 8332507: compilation result depends on compilation order --- .../com/sun/tools/javac/jvm/ClassReader.java | 55 ++++- ...ildcardBoundsNotReadFromClassFileTest.java | 194 ++++++++++++++++++ 2 files changed, 248 insertions(+), 1 deletion(-) create mode 100644 test/langtools/tools/javac/generics/wildcards/separate_compilation/WildcardBoundsNotReadFromClassFileTest.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index e0cebce611de0..f87cd0e35ec42 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -30,10 +30,12 @@ import java.net.URISyntaxException; import java.nio.CharBuffer; import java.nio.file.ClosedFileSystemException; +import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; @@ -590,7 +592,24 @@ Type classSigToType() { ClassSymbol t = enterClass(readName(signatureBuffer, startSbp, sbp - startSbp)); - outer = new ClassType(outer, sigToTypes('>'), t) { + List actuals = sigToTypes('>'); + List formals = ((ClassType)t.type.tsym.type).typarams_field; + if (formals != null) { + if (actuals.isEmpty()) + actuals = formals; + else + updateBounds(actuals, formals); + } + if (actuals.length() != (formals == null ? 0 : formals.length())) { + final List actualsCp = actuals; + addSymbolReadListener(t, () -> { + List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; + if (formalsCp != null && !formalsCp.isEmpty()) { + updateBounds(actualsCp, formalsCp); + } + }); + } + outer = new ClassType(outer, actuals, t) { boolean completed = false; @Override @DefinedBy(Api.LANGUAGE_MODEL) public Type getEnclosingType() { @@ -667,6 +686,18 @@ public void setEnclosingType(Type outer) { } } + private void updateBounds(List actuals, List formals) { + if (actuals.length() == formals.length()) { + List a = actuals; + List f = formals; + while (a.nonEmpty()) { + a.head = a.head.withTypeVar(f.head); + a = a.tail; + f = f.tail; + } + } + } + /** Quote a bogus signature for display inside an error message. */ String quoteBadSignature() { @@ -2877,6 +2908,21 @@ protected ClassSymbol enterClass(Name name, TypeSymbol owner) { return syms.enterClass(currentModule, name, owner); } + Map> symbolReadListeners = new LinkedHashMap<>(); + + interface SymbolReadListener { + void symbolRead(); + } + + private void addSymbolReadListener(ClassSymbol csym, SymbolReadListener listener) { + java.util.List listeners = symbolReadListeners.get(csym); + if (listeners == null) { + listeners = new ArrayList<>(); + } + listeners.add(listener); + symbolReadListeners.put(csym, listeners); + } + /** Read contents of a given class symbol `c'. Both external and internal * versions of an inner class are read. */ @@ -2962,6 +3008,13 @@ void readClass(ClassSymbol c) { } } typevars = typevars.leave(); + java.util.List listeners = symbolReadListeners.get(c); + if (listeners != null) { + for (SymbolReadListener listener : listeners) { + listener.symbolRead(); + } + symbolReadListeners.remove(c); + } } private MethodSymbol lookupMethod(TypeSymbol tsym, Name name, List argtypes) { diff --git a/test/langtools/tools/javac/generics/wildcards/separate_compilation/WildcardBoundsNotReadFromClassFileTest.java b/test/langtools/tools/javac/generics/wildcards/separate_compilation/WildcardBoundsNotReadFromClassFileTest.java new file mode 100644 index 0000000000000..6d44f7ccf1e4e --- /dev/null +++ b/test/langtools/tools/javac/generics/wildcards/separate_compilation/WildcardBoundsNotReadFromClassFileTest.java @@ -0,0 +1,194 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8332507 + * @summary compilation result depends on compilation order + * @library /tools/lib + * @modules + * jdk.compiler/com.sun.tools.javac.code + * jdk.compiler/com.sun.tools.javac.util + * jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.file + * jdk.compiler/com.sun.tools.javac.main + * jdk.jdeps/com.sun.tools.classfile + * @build toolbox.ToolBox toolbox.JavacTask + * @run main WildcardBoundsNotReadFromClassFileTest + */ + +import java.nio.file.Path; +import java.nio.file.Paths; + +import com.sun.tools.javac.code.Flags; +import com.sun.tools.javac.util.Assert; +import com.sun.tools.classfile.ClassFile; + +import toolbox.TestRunner; +import toolbox.ToolBox; +import toolbox.JavacTask; +import toolbox.Task; + +public class WildcardBoundsNotReadFromClassFileTest extends TestRunner { + ToolBox tb = new ToolBox(); + + public WildcardBoundsNotReadFromClassFileTest() { + super(System.err); + } + + protected void runTests() throws Exception { + runTests(m -> new Object[] { Paths.get(m.getName()) }); + } + + Path[] findJavaFiles(Path... paths) throws Exception { + return tb.findJavaFiles(paths); + } + + public static void main(String... args) throws Exception { + new WildcardBoundsNotReadFromClassFileTest().runTests(); + } + + @Test + public void testSeparateCompilation1(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, + """ + import java.util.List; + public class A { + static void of(List> attributes) {} + } + class Attribute> {} + class A1 extends Attribute {} + """); + Path classes = base.resolve("classes"); + tb.createDirectories(classes); + + // let's compile A.java first + new toolbox.JavacTask(tb) + .outdir(classes) + .files(findJavaFiles(src)) + .run() + .writeAll(); + + // now class Test with the rest in the classpath + tb.writeJavaFiles(src, + """ + import java.util.List; + import java.util.stream.*; + public class Test { + void m(Stream> stream, boolean cond) { + A.of(stream.map(atr -> cond ? (A1) atr : atr).toList()); + } + } + """); + new toolbox.JavacTask(tb) + .outdir(classes) + .options("-cp", classes.toString()) + .files(src.resolve("Test.java")) + .run() + .writeAll(); + } + + @Test + public void testSeparateCompilation2(Path base) throws Exception { + // this test uses nested classes + Path src = base.resolve("src"); + tb.writeJavaFiles(src, + """ + import java.util.List; + public class A { + static void of(List> attributes) {} + public interface Attribute> { + public static final class A1 implements Attribute {} + } + } + """); + Path classes = base.resolve("classes"); + tb.createDirectories(classes); + + // let's compile A.java first + new toolbox.JavacTask(tb) + .outdir(classes) + .files(findJavaFiles(src)) + .run() + .writeAll(); + + // now class Test with the rest in the classpath + tb.writeJavaFiles(src, + """ + import java.util.List; + import java.util.stream.*; + public class Test { + void m(Stream> stream, boolean cond) { + A.of(stream.map(atr -> cond ? (A.Attribute.A1) atr : atr).toList()); + } + } + """); + new toolbox.JavacTask(tb) + .outdir(classes) + .options("-cp", classes.toString()) + .files(src.resolve("Test.java")) + .run() + .writeAll(); + } + + @Test + public void testSeparateCompilation3(Path base) throws Exception { + // this test uses nested classes too + Path src = base.resolve("src"); + tb.writeJavaFiles(src, + """ + import java.util.Map; + abstract class A { + interface I {} + abstract void f(Map> i); + } + """); + Path classes = base.resolve("classes"); + tb.createDirectories(classes); + + // let's compile A.java first + new toolbox.JavacTask(tb) + .outdir(classes) + .files(findJavaFiles(src)) + .run() + .writeAll(); + + // now class B with the rest in the classpath + tb.writeJavaFiles(src, + """ + import java.util.Map; + public class B { + void f(A a, Map> x) { + a.f(x); + } + } + """); + new toolbox.JavacTask(tb) + .outdir(classes) + .options("-cp", classes.toString()) + .files(src.resolve("B.java")) + .run() + .writeAll(); + } +} From 81ebe2961a7b2c6ae717bef71ccfdddeea68ec86 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Tue, 28 May 2024 11:19:11 -0400 Subject: [PATCH 2/6] addressing review comments, levarage the existing complete hooks --- .../com/sun/tools/javac/jvm/ClassReader.java | 43 ++++++------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index f87cd0e35ec42..896c923ab3f4f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -600,15 +600,12 @@ Type classSigToType() { else updateBounds(actuals, formals); } - if (actuals.length() != (formals == null ? 0 : formals.length())) { - final List actualsCp = actuals; - addSymbolReadListener(t, () -> { - List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; - if (formalsCp != null && !formalsCp.isEmpty()) { - updateBounds(actualsCp, formalsCp); - } - }); - } + /* actualsCp is final as it will be captured by the inner class below. We could avoid defining + * this additional local variable and depend on field ClassType::typarams_field which `actuals` is + * assigned to but then we would have a dependendy on the internal representation of ClassType which + * could change in the future + */ + final List actualsCp = actuals; outer = new ClassType(outer, actuals, t) { boolean completed = false; @Override @DefinedBy(Api.LANGUAGE_MODEL) @@ -633,6 +630,12 @@ public Type getEnclosingType() { } else { super.setEnclosingType(Type.noType); } + if (actualsCp.length() != (formals == null ? 0 : formals.length())) { + List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; + if (formalsCp != null && !formalsCp.isEmpty()) { + updateBounds(actualsCp, formalsCp); + } + } } return super.getEnclosingType(); } @@ -2908,21 +2911,6 @@ protected ClassSymbol enterClass(Name name, TypeSymbol owner) { return syms.enterClass(currentModule, name, owner); } - Map> symbolReadListeners = new LinkedHashMap<>(); - - interface SymbolReadListener { - void symbolRead(); - } - - private void addSymbolReadListener(ClassSymbol csym, SymbolReadListener listener) { - java.util.List listeners = symbolReadListeners.get(csym); - if (listeners == null) { - listeners = new ArrayList<>(); - } - listeners.add(listener); - symbolReadListeners.put(csym, listeners); - } - /** Read contents of a given class symbol `c'. Both external and internal * versions of an inner class are read. */ @@ -3008,13 +2996,6 @@ void readClass(ClassSymbol c) { } } typevars = typevars.leave(); - java.util.List listeners = symbolReadListeners.get(c); - if (listeners != null) { - for (SymbolReadListener listener : listeners) { - listener.symbolRead(); - } - symbolReadListeners.remove(c); - } } private MethodSymbol lookupMethod(TypeSymbol tsym, Name name, List argtypes) { From ba4d99fe2ad877a6dcf01a6d637fb148b1a5a872 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Tue, 28 May 2024 11:21:50 -0400 Subject: [PATCH 3/6] removing unused imports --- .../share/classes/com/sun/tools/javac/jvm/ClassReader.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index 896c923ab3f4f..48bec27565a49 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -30,12 +30,10 @@ import java.net.URISyntaxException; import java.nio.CharBuffer; import java.nio.file.ClosedFileSystemException; -import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; From 6014d8c8d8b9bf1f86aa1e871deb923d6eca6d89 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Fri, 31 May 2024 09:54:03 -0400 Subject: [PATCH 4/6] review comments --- .../com/sun/tools/javac/jvm/ClassReader.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index 48bec27565a49..7b11dffd7ebb0 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -595,8 +595,6 @@ Type classSigToType() { if (formals != null) { if (actuals.isEmpty()) actuals = formals; - else - updateBounds(actuals, formals); } /* actualsCp is final as it will be captured by the inner class below. We could avoid defining * this additional local variable and depend on field ClassType::typarams_field which `actuals` is @@ -628,12 +626,6 @@ public Type getEnclosingType() { } else { super.setEnclosingType(Type.noType); } - if (actualsCp.length() != (formals == null ? 0 : formals.length())) { - List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; - if (formalsCp != null && !formalsCp.isEmpty()) { - updateBounds(actualsCp, formalsCp); - } - } } return super.getEnclosingType(); } @@ -641,7 +633,16 @@ public Type getEnclosingType() { public void setEnclosingType(Type outer) { throw new UnsupportedOperationException(); } - }; + + @Override + public List getTypeArguments() { + List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; + if (formalsCp != null && !formalsCp.isEmpty()) { + updateBounds(actualsCp, formalsCp); + } + return super.getTypeArguments(); + } + }; switch (signature[sigp++]) { case ';': if (sigp < siglimit && signature[sigp] == '.') { From b22d7417cc370d3f19072b3d5d7f24b7c92c844c Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Fri, 31 May 2024 10:03:58 -0400 Subject: [PATCH 5/6] review comments --- .../com/sun/tools/javac/jvm/ClassReader.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index 7b11dffd7ebb0..310828c50945f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -638,7 +638,15 @@ public void setEnclosingType(Type outer) { public List getTypeArguments() { List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; if (formalsCp != null && !formalsCp.isEmpty()) { - updateBounds(actualsCp, formalsCp); + if (actualsCp.length() == formalsCp.length()) { + List a = actualsCp; + List f = formalsCp; + while (a.nonEmpty()) { + a.head = a.head.withTypeVar(f.head); + a = a.tail; + f = f.tail; + } + } } return super.getTypeArguments(); } @@ -688,18 +696,6 @@ public List getTypeArguments() { } } - private void updateBounds(List actuals, List formals) { - if (actuals.length() == formals.length()) { - List a = actuals; - List f = formals; - while (a.nonEmpty()) { - a.head = a.head.withTypeVar(f.head); - a = a.tail; - f = f.tail; - } - } - } - /** Quote a bogus signature for display inside an error message. */ String quoteBadSignature() { From d88e8555cca8613fc00796497cccf1985507ca22 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Fri, 31 May 2024 10:50:05 -0400 Subject: [PATCH 6/6] review comments --- .../com/sun/tools/javac/jvm/ClassReader.java | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java index 310828c50945f..22b21b86b9b16 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java @@ -604,6 +604,7 @@ Type classSigToType() { final List actualsCp = actuals; outer = new ClassType(outer, actuals, t) { boolean completed = false; + boolean typeArgsSet = false; @Override @DefinedBy(Api.LANGUAGE_MODEL) public Type getEnclosingType() { if (!completed) { @@ -636,15 +637,18 @@ public void setEnclosingType(Type outer) { @Override public List getTypeArguments() { - List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; - if (formalsCp != null && !formalsCp.isEmpty()) { - if (actualsCp.length() == formalsCp.length()) { - List a = actualsCp; - List f = formalsCp; - while (a.nonEmpty()) { - a.head = a.head.withTypeVar(f.head); - a = a.tail; - f = f.tail; + if (!typeArgsSet) { + typeArgsSet = true; + List formalsCp = ((ClassType)t.type.tsym.type).typarams_field; + if (formalsCp != null && !formalsCp.isEmpty()) { + if (actualsCp.length() == formalsCp.length()) { + List a = actualsCp; + List f = formalsCp; + while (a.nonEmpty()) { + a.head = a.head.withTypeVar(f.head); + a = a.tail; + f = f.tail; + } } } }