From d1540e2a49c7a41eb771fc9896c367187d070dec Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Thu, 24 Oct 2024 17:25:43 +0000 Subject: [PATCH] 8342090: Infer::IncorporationBinaryOp::equals can produce side-effects 8288590: javac failure: incompatible types: cannot infer type arguments due to Object.hashCode collision Co-authored-by: Maurizio Cimadamore Reviewed-by: mcimadamore --- .../com/sun/tools/javac/code/Types.java | 4 + .../com/sun/tools/javac/comp/Infer.java | 54 +++++--------- .../tools/javac/comp/InferenceContext.java | 14 ++++ .../NonDeterminismTest.java | 74 +++++++++++++++++++ 4 files changed, 111 insertions(+), 35 deletions(-) create mode 100644 test/langtools/tools/javac/inference_non_determinism/NonDeterminismTest.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java index aa71c9446dc..e35ecbdc956 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java @@ -3328,6 +3328,10 @@ public class Types { return t.map(new Subst(from, to)); } + /* this class won't substitute all types for example UndetVars are never substituted, this is + * by design as UndetVars are used locally during inference and shouldn't escape from inference routines, + * some specialized applications could need a tailored solution + */ private class Subst extends StructuralTypeMapping { List from; List to; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java index f5df6adddf3..81e07277bfb 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java @@ -801,15 +801,15 @@ public class Infer { /** * Helper function: perform subtyping through incorporation cache. */ - boolean isSubtype(Type s, Type t, Warner warn) { - return doIncorporationOp(IncorporationBinaryOpKind.IS_SUBTYPE, s, t, warn); + boolean isSubtype(Type s, Type t, Warner warn, InferenceContext ic) { + return doIncorporationOp(IncorporationBinaryOpKind.IS_SUBTYPE, s, t, warn, ic); } /** * Helper function: perform type-equivalence through incorporation cache. */ - boolean isSameType(Type s, Type t) { - return doIncorporationOp(IncorporationBinaryOpKind.IS_SAME_TYPE, s, t, null); + boolean isSameType(Type s, Type t, InferenceContext ic) { + return doIncorporationOp(IncorporationBinaryOpKind.IS_SAME_TYPE, s, t, null, ic); } @Override @@ -853,7 +853,7 @@ public class Infer { for (Type b : uv.getBounds(to)) { b = typeFunc.apply(inferenceContext, b); if (optFilter != null && optFilter.test(inferenceContext, b)) continue; - boolean success = checkBound(t, b, from, to, warn); + boolean success = checkBound(t, b, from, to, warn, inferenceContext); if (!success) { report(from, to); } @@ -873,13 +873,13 @@ public class Infer { /** * Is source type 's' compatible with target type 't' given source and target bound kinds? */ - boolean checkBound(Type s, Type t, InferenceBound ib_s, InferenceBound ib_t, Warner warn) { + boolean checkBound(Type s, Type t, InferenceBound ib_s, InferenceBound ib_t, Warner warn, InferenceContext ic) { if (ib_s.lessThan(ib_t)) { - return isSubtype(s, t, warn); + return isSubtype(s, t, warn, ic); } else if (ib_t.lessThan(ib_s)) { - return isSubtype(t, s, warn); + return isSubtype(t, s, warn, ic); } else { - return isSameType(s, t); + return isSameType(s, t, ic); } } @@ -1010,7 +1010,7 @@ public class Infer { if (!allParamsSuperBound1.head.hasTag(WILDCARD) && !allParamsSuperBound2.head.hasTag(WILDCARD)) { if (!isSameType(inferenceContext.asUndetVar(allParamsSuperBound1.head), - inferenceContext.asUndetVar(allParamsSuperBound2.head))) { + inferenceContext.asUndetVar(allParamsSuperBound2.head), inferenceContext)) { reportBoundError(uv, InferenceBound.UPPER); } } @@ -1194,11 +1194,11 @@ public class Infer { types.asSuper(t, sup.tsym); } - boolean doIncorporationOp(IncorporationBinaryOpKind opKind, Type op1, Type op2, Warner warn) { - IncorporationBinaryOp newOp = new IncorporationBinaryOp(opKind, op1, op2); + boolean doIncorporationOp(IncorporationBinaryOpKind opKind, Type op1, Type op2, Warner warn, InferenceContext ic) { + IncorporationBinaryOpKey newOp = new IncorporationBinaryOpKey(opKind, ic.asTypeVar(op1), ic.asTypeVar(op2), types); Boolean res = incorporationCache.get(newOp); if (res == null) { - incorporationCache.put(newOp, res = newOp.apply(warn)); + incorporationCache.put(newOp, res = opKind.apply(op1, op2, warn, types)); } return res; } @@ -1232,26 +1232,14 @@ public class Infer { * are not executed unnecessarily (which would potentially lead to adding * same bounds over and over). */ - class IncorporationBinaryOp { - - IncorporationBinaryOpKind opKind; - Type op1; - Type op2; - - IncorporationBinaryOp(IncorporationBinaryOpKind opKind, Type op1, Type op2) { - this.opKind = opKind; - this.op1 = op1; - this.op2 = op2; - } - + record IncorporationBinaryOpKey(IncorporationBinaryOpKind opKind, Type op1, Type op2, Types types) { @Override public boolean equals(Object o) { - return (o instanceof IncorporationBinaryOp incorporationBinaryOp) - && opKind == incorporationBinaryOp.opKind - && types.isSameType(op1, incorporationBinaryOp.op1) - && types.isSameType(op2, incorporationBinaryOp.op2); + return (o instanceof IncorporationBinaryOpKey anotherKey) + && opKind == anotherKey.opKind + && types.isSameType(op1, anotherKey.op1) + && types.isSameType(op2, anotherKey.op2); } - @Override public int hashCode() { int result = opKind.hashCode(); @@ -1261,14 +1249,10 @@ public class Infer { result += types.hashCode(op2); return result; } - - boolean apply(Warner warn) { - return opKind.apply(op1, op2, warn, types); - } } /** an incorporation cache keeps track of all executed incorporation-related operations */ - Map incorporationCache = new LinkedHashMap<>(); + Map incorporationCache = new LinkedHashMap<>(); protected static class BoundFilter implements Predicate { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/InferenceContext.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/InferenceContext.java index 2fec365aff5..35cd9a25ae4 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/InferenceContext.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/InferenceContext.java @@ -214,6 +214,20 @@ public class InferenceContext { return buf.toList(); } + /** + * Replace all undet vars in a given type with corresponding free variables + */ + public final Type asTypeVar(Type t) { + return asTypeVarFun.apply(t); + } + + Types.TypeMapping asTypeVarFun = new Type.StructuralTypeMapping<>() { + @Override + public Type visitUndetVar(UndetVar uv, Void aVoid) { + return uv.qtype; + } + }; + List instTypes() { ListBuffer buf = new ListBuffer<>(); for (Type t : undetvars) { diff --git a/test/langtools/tools/javac/inference_non_determinism/NonDeterminismTest.java b/test/langtools/tools/javac/inference_non_determinism/NonDeterminismTest.java new file mode 100644 index 00000000000..2f245d4f877 --- /dev/null +++ b/test/langtools/tools/javac/inference_non_determinism/NonDeterminismTest.java @@ -0,0 +1,74 @@ +/* + * 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 8342090 8288590 + * @summary Infer::IncorporationBinaryOp::equals can produce side-effects + * @compile NonDeterminismTest.java + * @compile -J-XX:+UnlockExperimentalVMOptions -J-XX:hashCode=2 NonDeterminismTest.java + */ + +import java.util.*; +import java.lang.foreign.*; +import static java.lang.foreign.ValueLayout.*; + +import static java.util.Arrays.asList; + +class NonDeterminismTest { + void test1() { + Map CANONICAL_LAYOUTS = Map.ofEntries( + // specified canonical layouts + Map.entry("bool", JAVA_BOOLEAN), + Map.entry("char", JAVA_BYTE), + Map.entry("float", JAVA_FLOAT), + Map.entry("long long", JAVA_LONG), + Map.entry("double", JAVA_DOUBLE), + Map.entry("void*", ADDRESS), + // JNI types + Map.entry("jboolean", JAVA_BOOLEAN), + Map.entry("jchar", JAVA_CHAR), + Map.entry("jbyte", JAVA_BYTE), + Map.entry("jshort", JAVA_SHORT), + Map.entry("jint", JAVA_INT), + Map.entry("jlong", JAVA_LONG), + Map.entry("jfloat", JAVA_FLOAT), + Map.entry("jdouble", JAVA_DOUBLE) + ); + } + + class Test2 { + interface I1 {} + interface I2 {} + + record R1(List T1) implements I1 {} + record R2(List T1, List T2) implements I2 {} + + I1 m1(T1 T1) { + return new R1<>(asList(T1)); + } + I2 m2(T1 T1, T2 T2) { + return new R2<>(asList(T1), asList(T2)); + } + } +}