ECJ fails to compile method call with 2 method references with cycling

generics whilst javac succeeds

+ implement IC18.pickFromCycle with fresh interpretation of JLS (25)

Fixes eclipse-jdt#4346

Author: stephan-herrmann

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/InferenceContext18.java

@@ -1408,19 +1408,15 @@ private void addDependencies(BoundSet boundSet, Set<InferenceVariable> variableS
 	}
 
 	private ConstraintFormula pickFromCycle(Set<ConstraintFormula> c) {
-		// Detail from 18.5.2 bullet 6.1
-
 		// Note on performance: this implementation could quite possibly be optimized a lot.
 		// However, we only *very rarely* reach here,
 		// so nobody should really be affected by the performance penalty paid here.
 
-		// Note on spec conformance: the spec seems to require _all_ criteria (i)-(iv) to be fulfilled
-		// with the sole exception of (iii), which should only be used, if _any_ constraints matching (i) & (ii)
-		// also fulfill this condition.
-		// Experiments, however, show that strict application of the above is prone to failing to pick any constraint,
-		// causing non-termination of the algorithm.
-		// Since that is not acceptable, I'm *interpreting* the spec to request a search for a constraint
-		// that "best matches" the given conditions.
+		// from JLS 18.5.2.2 bullet 3.1 para 2:
+		//
+		// If this subset is empty, then there is a cycle (or cycles) in the graph of dependencies between constraints.
+		// In this case, the constraints in C that participate in a dependency cycle (or cycles) and
+		// do not depend on any constraints outside of the cycle (or cycles) are considered.
 
 		// collect all constraints participating in a cycle
 		HashMap<ConstraintFormula,Set<ConstraintFormula>> dependencies = new HashMap<>();
@@ -1448,10 +1444,10 @@ private ConstraintFormula pickFromCycle(Set<ConstraintFormula> c) {
 		outside.removeAll(cycles);
 
 		Set<ConstraintFormula> candidatesII = new LinkedHashSet<>();
-		// (i): participates in a cycle:
+		// participates in a cycle:
 		candidates: for (ConstraintFormula candidate : cycles) {
 			Collection<InferenceVariable> infVars = candidate.inputVariables(this);
-			// (ii) does not depend on any constraints outside the cycle
+			// does not depend on any constraints outside the cycle
 			for (ConstraintFormula out : outside) {
 				if (dependsOn(infVars, out.outputVariables(this)))
 					continue candidates;
@@ -1461,62 +1457,39 @@ private ConstraintFormula pickFromCycle(Set<ConstraintFormula> c) {
 		if (candidatesII.isEmpty())
 			candidatesII = c; // not spec'ed but needed to avoid returning null below, witness: java.util.stream.Collectors
 
-		// tentatively: (iii)  has the form ⟨Expression → T⟩
-		Set<ConstraintFormula> candidatesIII = new LinkedHashSet<>();
+		// JLS cntd:
+		// A single constraint is selected from these considered constraints, as follows:
+		// * If any of the considered constraints have the form ‹Expression → T›, then
+		//   the selected constraint is the considered constraint of this form that contains the expression
+		//   to the left (§3.5) of the expression of every other considered constraint of this form.
+		int minStart = Integer.MAX_VALUE;
+		ConstraintFormula leftMost = null;
 		for (ConstraintFormula candidate : candidatesII) {
-			if (candidate instanceof ConstraintExpressionFormula)
-				candidatesIII.add(candidate);
-		}
-		if (candidatesIII.isEmpty()) {
-			candidatesIII = candidatesII; // no constraint fulfills (iii) -> ignore this condition
-		} else { // candidatesIII contains all relevant constraints ⟨Expression → T⟩
-			// (iv) contains an expression that appears to the left of the expression
-			// 		of every other constraint satisfying the previous three requirements
-
-			// collect containment info regarding all expressions in candidate constraints:
-			// (a) find minimal enclosing expressions:
-			Map<ConstraintExpressionFormula,ConstraintExpressionFormula> expressionContainedBy = new LinkedHashMap<>();
-			for (ConstraintFormula one : candidatesIII) {
-				ConstraintExpressionFormula oneCEF = (ConstraintExpressionFormula) one;
-				Expression exprOne = oneCEF.left;
-				for (ConstraintFormula two : candidatesIII) {
-					if (one == two) continue;
-					ConstraintExpressionFormula twoCEF = (ConstraintExpressionFormula) two;
-					Expression exprTwo = twoCEF.left;
-					if (doesExpressionContain(exprOne, exprTwo)) {
-						ConstraintExpressionFormula previous = expressionContainedBy.get(two);
-						if (previous == null || doesExpressionContain(previous.left, exprOne)) // only if improving
-							expressionContainedBy.put(twoCEF, oneCEF);
-					}
-				}
-			}
-			// (b) build the tree from the above
-			Map<ConstraintExpressionFormula,Set<ConstraintExpressionFormula>> containmentForest = new LinkedHashMap<>();
-			for (Map.Entry<ConstraintExpressionFormula, ConstraintExpressionFormula> parentRelation : expressionContainedBy.entrySet()) {
-				ConstraintExpressionFormula parent = parentRelation.getValue();
-				Set<ConstraintExpressionFormula> children = containmentForest.get(parent);
-				if (children == null)
-					containmentForest.put(parent, children = new LinkedHashSet<>());
-				children.add(parentRelation.getKey());
+			if (candidate instanceof ConstraintExpressionFormula cef && cef.left.sourceStart < minStart) {
+				minStart = cef.left.sourceStart;
+				leftMost = cef;
 			}
+		}
+		if (leftMost != null)
+			return leftMost;
 
-			// approximate the spec by searching the largest containment tree:
-			int bestRank = -1;
-			ConstraintExpressionFormula candidate = null;
-			for (ConstraintExpressionFormula parent : containmentForest.keySet()) {
-				int rank = rankNode(parent, expressionContainedBy, containmentForest);
-				if (rank > bestRank) {
-					bestRank = rank;
-					candidate = parent;
-				}
+		// JLS cntd:
+		// * If no considered constraint has the form ‹Expression → T›, then the selected constraint is the considered
+		//   constraint that contains the expression to the left of the expression of every other considered constraint.
+		for (ConstraintFormula candidate : candidatesII) {
+			// note: ConstraintExpressionFormula is the only shape left, which contains an expression
+			if (candidate instanceof ConstraintExceptionFormula cef && cef.left.sourceStart < minStart) {
+				minStart = cef.left.sourceStart;
+				leftMost = cef;
 			}
-			if (candidate != null)
-				return candidate;
 		}
+		if (leftMost != null)
+			return leftMost;
 
-		if (candidatesIII.isEmpty())
+		// not spec'ed: random pick
+		if (candidatesII.isEmpty())
 			throw new IllegalStateException("cannot pick constraint from cyclic set"); //$NON-NLS-1$
-		return candidatesIII.iterator().next();
+		return candidatesII.iterator().next();
 	}
 
 	/**
@@ -1554,35 +1527,6 @@ private boolean isReachable(Map<ConstraintFormula,Set<ConstraintFormula>> deps,
 		return false;
 	}
 
-	/** Does exprOne lexically contain exprTwo? */
-	private boolean doesExpressionContain(Expression exprOne, Expression exprTwo) {
-		if (exprTwo.sourceStart > exprOne.sourceStart) {
-			return exprTwo.sourceEnd <= exprOne.sourceEnd;
-		} else if (exprTwo.sourceStart == exprOne.sourceStart) {
-			return exprTwo.sourceEnd < exprOne.sourceEnd;
-		}
-		return false;
-	}
-
-	/** non-roots answer -1, roots answer the size of the spanned tree */
-	private int rankNode(ConstraintExpressionFormula parent,
-			Map<ConstraintExpressionFormula,ConstraintExpressionFormula> expressionContainedBy,
-			Map<ConstraintExpressionFormula, Set<ConstraintExpressionFormula>> containmentForest)
-	{
-		if (expressionContainedBy.get(parent) != null)
-			return -1; // not a root
-		Set<ConstraintExpressionFormula> children = containmentForest.get(parent);
-		if (children == null)
-			return 1; // unconnected node or leaf
-		int sum = 1;
-		for (ConstraintExpressionFormula child : children) {
-			int cRank = rankNode(child, expressionContainedBy, containmentForest);
-			if (cRank > 0)
-				sum += cRank;
-		}
-		return sum;
-	}
-
 	private Set<ConstraintFormula> findBottomSet(Set<ConstraintFormula> constraints,
 			Set<InferenceVariable> allOutputVariables, List<Set<InferenceVariable>> components)
 	{

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest_9.java

@@ -1559,6 +1559,28 @@ public static abstract class AbstractJsonNodeMatcher<A extends JsonNode> impleme
 		},
 		"");
 }
+public void testGH4346() {
+	runConformTest(new String[] {
+			"X.java",
+			"""
+			import java.util.function.Function;
+			public class X {
+				public static <T> W1<W2<T>> main(String[] args) {
+					return m(m2(), W2::t, W2::new);
+				}
+
+				public static <T1, T2> W1<T1> m(W1<T2> w1, Function<T1, T2> f1, Function<T2, T1> f2) {
+					return null;
+				}
+				private static <T> W1<W1<T>> m2() {
+					return null;
+				}
+				private record W1<T>(T t) {}
+				private record W2<T>(W1<T> t) {}
+			}
+			"""
+	});
+}
 public static Class<GenericsRegressionTest_9> testClass() {
 	return GenericsRegressionTest_9.class;
 }