Skip to content

Commit ba4e639

Browse files
authored
Type argument inference fixes. (#7002)
Co-authored-by: Michael Ernst <mernst@cs.washington.edu> Fixes #6891.
1 parent 51f6d6b commit ba4e639

File tree

7 files changed

+266
-77
lines changed

7 files changed

+266
-77
lines changed

framework/src/main/java/org/checkerframework/framework/util/typeinference8/InvocationTypeInference.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,6 @@ public ConstraintSet createC(
462462
c.addAll(aa.reduce(context));
463463
}
464464
} else {
465-
// Wait to reduce additional argument constraints from lambdas and method references
466-
// because the additional constraints might require other inference variables to be
467-
// resolved before the constraint can be created.
468465
c.addAll(createAdditionalArgConstraints(ei, fi, map));
469466
}
470467
}
@@ -513,7 +510,7 @@ private ConstraintSet createAdditionalArgConstraints(
513510
case METHOD_INVOCATION:
514511
case NEW_CLASS:
515512
if (TreeUtils.isPolyExpression(ei)) {
516-
c.add(new AdditionalArgument(ei));
513+
c.addAll(new AdditionalArgument(ei).reduce(context));
517514
}
518515
break;
519516
case PARENTHESIZED:
@@ -564,7 +561,15 @@ private ConstraintSet createAdditionalArgConstraintsNoLambda(ExpressionTree expr
564561
case METHOD_INVOCATION:
565562
case NEW_CLASS:
566563
if (TreeUtils.isPolyExpression(expression)) {
567-
c.add(new AdditionalArgument(expression));
564+
try {
565+
c.addAll(new AdditionalArgument(expression).reduce(context));
566+
} catch (Exception e) {
567+
// Sometimes in order to create the additional argument constraint, other inference
568+
// variables must be resolved first. This happens when a lambda parameter is used in the
569+
// additional argument constraint.
570+
// See framework/tests/all-systems/SimpleLambdaParameter.java
571+
c.add(new AdditionalArgument(expression));
572+
}
568573
}
569574
break;
570575
case PARENTHESIZED:
@@ -666,7 +671,7 @@ private BoundSet getB4(BoundSet b3, ConstraintSet c) {
666671
Set<Variable> newVariables = c.getAllInferenceVariables();
667672
while (!c.isEmpty()) {
668673

669-
ConstraintSet subset = c.getClosedSubset(b3.getDependencies(newVariables));
674+
ConstraintSet subset = ConstraintSet.getClosedSubset(c, b3.getDependencies(newVariables));
670675
Set<Variable> alphas = subset.getAllInputVariables();
671676
if (!alphas.isEmpty()) {
672677
// First resolve only the variables with proper bounds.

framework/src/main/java/org/checkerframework/framework/util/typeinference8/bound/BoundSet.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,8 @@ public Dependencies getDependencies(Collection<Variable> additionalVars) {
272272
Set<Variable> allVariables = new LinkedHashSet<>(variables);
273273
allVariables.addAll(additionalVars);
274274
for (Variable alpha : allVariables) {
275-
LinkedHashSet<Variable> alphaDependencies = new LinkedHashSet<>();
276-
// An inference variable alpha depends on the resolution of itself.
277-
alphaDependencies.add(alpha);
278-
alphaDependencies.addAll(alpha.getBounds().getVariablesMentionedInBounds());
275+
LinkedHashSet<Variable> alphaDependencies =
276+
new LinkedHashSet<>(alpha.getBounds().getVariablesMentionedInBounds());
279277

280278
if (alpha.isCaptureVariable()) {
281279
// If alpha appears on the left-hand side of another bound of the form
@@ -292,6 +290,8 @@ public Dependencies getDependencies(Collection<Variable> additionalVars) {
292290
}
293291
}
294292
}
293+
// An inference variable alpha depends on the resolution of itself.
294+
dependencies.putOrAdd(alpha, alpha);
295295
}
296296

297297
// Add transitive dependencies

framework/src/main/java/org/checkerframework/framework/util/typeinference8/constraint/CheckedExceptionConstraint.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
package org.checkerframework.framework.util.typeinference8.constraint;
22

33
import com.sun.source.tree.ExpressionTree;
4+
import com.sun.source.tree.LambdaExpressionTree;
5+
import com.sun.source.tree.MemberReferenceTree;
46
import com.sun.source.tree.Tree;
57
import java.util.ArrayList;
8+
import java.util.Collections;
69
import java.util.List;
710
import java.util.Objects;
11+
import javax.lang.model.type.TypeKind;
812
import org.checkerframework.framework.util.typeinference8.types.AbstractType;
13+
import org.checkerframework.framework.util.typeinference8.types.UseOfVariable;
914
import org.checkerframework.framework.util.typeinference8.types.Variable;
1015
import org.checkerframework.framework.util.typeinference8.util.Java8InferenceContext;
1116
import org.checkerframework.framework.util.typeinference8.util.Theta;
17+
import org.checkerframework.javacutil.TreeUtils;
1218

1319
/**
1420
* &lt;LambdaExpression &rarr;throws T&gt;: The checked exceptions thrown by the body of the
@@ -53,6 +59,52 @@ public Kind getKind() {
5359

5460
@Override
5561
public List<Variable> getInputVariables() {
62+
if (getKind() == Kind.LAMBDA_EXCEPTION) {
63+
if (T.isUseOfVariable()) {
64+
return Collections.singletonList(((UseOfVariable) T).getVariable());
65+
} else {
66+
LambdaExpressionTree lambdaTree = (LambdaExpressionTree) expression;
67+
List<Variable> inputs = new ArrayList<>();
68+
if (TreeUtils.isImplicitlyTypedLambda(lambdaTree)) {
69+
List<AbstractType> params = this.T.getFunctionTypeParameterTypes();
70+
if (params == null) {
71+
// T is not a function type.
72+
return Collections.emptyList();
73+
}
74+
for (AbstractType param : params) {
75+
inputs.addAll(param.getInferenceVariables());
76+
}
77+
}
78+
AbstractType R = this.T.getFunctionTypeReturnType();
79+
if (R == null || R.getTypeKind() == TypeKind.NONE) {
80+
return inputs;
81+
}
82+
inputs.addAll(R.getInferenceVariables());
83+
return inputs;
84+
}
85+
} else if (getKind() == Kind.METHOD_REF_EXCEPTION) {
86+
if (T.isUseOfVariable()) {
87+
return Collections.singletonList(((UseOfVariable) T).getVariable());
88+
} else if (TreeUtils.isExactMethodReference((MemberReferenceTree) expression)) {
89+
return Collections.emptyList();
90+
} else {
91+
List<AbstractType> params = this.T.getFunctionTypeParameterTypes();
92+
if (params == null) {
93+
// T is not a function type.
94+
return Collections.emptyList();
95+
}
96+
List<Variable> inputs = new ArrayList<>();
97+
for (AbstractType param : params) {
98+
inputs.addAll(param.getInferenceVariables());
99+
}
100+
AbstractType R = this.T.getFunctionTypeReturnType();
101+
if (R == null || R.getTypeKind() == TypeKind.NONE) {
102+
return inputs;
103+
}
104+
inputs.addAll(R.getInferenceVariables());
105+
return inputs;
106+
}
107+
}
56108
return getInputVariablesForExpression(expression, getT());
57109
}
58110

framework/src/main/java/org/checkerframework/framework/util/typeinference8/constraint/ConstraintSet.java

Lines changed: 110 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public String toString() {
5656
/**
5757
* A list of constraints in this set. It does not contain constraints that are equal. This needs
5858
* to be kept in the order created, which should be lexically left to right. This is so the {@link
59-
* #getClosedSubset(Dependencies)} is computed correctly.
59+
* #getClosedSubset(ConstraintSet, Dependencies)} is computed correctly.
6060
*/
6161
private final List<Constraint> list;
6262

@@ -139,6 +139,28 @@ public Constraint pop() {
139139
return list.remove(0);
140140
}
141141

142+
/**
143+
* Adds the constraint to the beginning of this set.
144+
*
145+
* @param constraint a constraint
146+
*/
147+
public void push(Constraint constraint) {
148+
if (constraint != null && !list.contains(constraint)) {
149+
list.add(0, constraint);
150+
}
151+
}
152+
153+
/**
154+
* Adds the constraints to the beginning of this set and maintains the order of the constraints.
155+
*
156+
* @param constraints constraints
157+
*/
158+
public void pushAll(ConstraintSet constraints) {
159+
for (int i = constraints.list.size() - 1; i > -1; i--) {
160+
this.push(constraints.list.get(i));
161+
}
162+
}
163+
142164
/**
143165
* Remove all constraints in {@code subset} from this constraint set.
144166
*
@@ -153,35 +175,61 @@ public void remove(ConstraintSet subset) {
153175
}
154176

155177
/**
156-
* A subset of constraints is selected in this constraint set, satisfying the property that, for
157-
* each constraint, no input variable can influence an output variable of another constraint in
158-
* this constraint set. (See JLS 18.5.2.2)
178+
* Returns a subset of {@code c}; for each constraint in the subset, no input variable can
179+
* influence an output variable of another constraint in C. If that subset is empty, returns a set
180+
* containing a single constraint that participates in a constraint cycle. (See JLS 18.5.2.2)
159181
*
182+
* @param c a constraint set
160183
* @param dependencies an object describing the dependencies of inference variables
161-
* @return s a subset of constraints is this constraint set
184+
* @return s a subset of constraints in {@code c} whose inputs do not affect {@code c}'s outputs,
185+
* or a singleton constraint from a constraint cycle
162186
*/
163-
public ConstraintSet getClosedSubset(Dependencies dependencies) {
187+
public static ConstraintSet getClosedSubset(ConstraintSet c, Dependencies dependencies) {
164188
ConstraintSet subset = new ConstraintSet();
165-
Set<Variable> inputDependencies = new LinkedHashSet<>();
166-
Set<Variable> outDependencies = new LinkedHashSet<>();
167-
for (Constraint constraint : list) {
189+
// Collection of all outputs of c.
190+
Set<Variable> allOutputsOfC = new LinkedHashSet<>();
191+
for (Constraint constraint : c.list) {
192+
if (constraint instanceof TypeConstraint) {
193+
allOutputsOfC.addAll(((TypeConstraint) constraint).getOutputVariables());
194+
}
195+
// No other constraints have output variables
196+
}
197+
198+
// From JLS 18.5.2.2:
199+
// A subset of constraints is selected in C, satisfying the property that, for each
200+
// constraint, no input variable can influence an output variable of another
201+
// constraint in C. The terms input variable and output variable are defined
202+
// below. An inference variable alpha can influence an inference variable beta if alpha
203+
// depends on the resolution of beta (§18.4), or vice versa; or if there exists a third
204+
// inference variable gamma such that alpha can influence gamma and gamma can influence beta.
205+
206+
// Put another way:
207+
// Find a subset of the set c where the following is true for all the constraints in the subset:
208+
// no input variable of a constraint can influence an output variable of any constraint in c.
209+
// (Influence means that neither variable can depend on the other.)
210+
// The JLS does not specify whether this subset should be as large as possible, but this
211+
// implementation returns the largest subset possible.
212+
for (Constraint constraint : c.list) {
168213
if (constraint.getKind() == Kind.EXPRESSION
169214
|| constraint.getKind() == Kind.LAMBDA_EXCEPTION
170215
|| constraint.getKind() == Kind.METHOD_REF_EXCEPTION) {
171-
TypeConstraint c = (TypeConstraint) constraint;
172-
Set<Variable> newInputs = dependencies.get(c.getInputVariables());
173-
Set<Variable> newOutputs = dependencies.get(c.getOutputVariables());
174-
if (Collections.disjoint(newInputs, outDependencies)
175-
&& Collections.disjoint(newOutputs, inputDependencies)) {
176-
inputDependencies.addAll(newInputs);
177-
outDependencies.addAll(newOutputs);
178-
subset.add(c);
179-
} else {
180-
// A cycle (or cycles) in the graph of dependencies between constraints exists.
181-
subset = new ConstraintSet();
182-
break;
216+
List<Variable> inputsOfSingleConstraint = ((TypeConstraint) constraint).getInputVariables();
217+
boolean foundInfluence = false;
218+
inputLoop:
219+
for (Variable in : inputsOfSingleConstraint) {
220+
for (Variable out : allOutputsOfC) {
221+
if (dependencies.get(in).contains(out) || dependencies.get(out).contains(in)) {
222+
foundInfluence = true;
223+
break inputLoop;
224+
}
225+
}
226+
}
227+
if (!foundInfluence) {
228+
// None of the inputs of constraint influence any output of any constraint in C.
229+
subset.add(constraint);
183230
}
184231
} else {
232+
// Other kinds of constraints do not have input variables.
185233
subset.add(constraint);
186234
}
187235
}
@@ -190,40 +238,47 @@ public ConstraintSet getClosedSubset(Dependencies dependencies) {
190238
return subset;
191239
}
192240

193-
outDependencies.clear();
194-
inputDependencies.clear();
195-
// If this subset is empty, then there is a cycle (or cycles) in the graph of dependencies
196-
// between constraints.
241+
// TODO: double check that this code is correct.
242+
// checker/tests/all-systems/java8inference/MapEntryGetFails.java is a test that uses this code.
243+
244+
Set<Variable> inputDependencies = new LinkedHashSet<>();
245+
Set<Variable> outDependencies = new LinkedHashSet<>();
246+
// If this subset is empty then no closed subset was found and there is a cycle (or cycles) in
247+
// the graph of dependencies between constraints.
248+
249+
// From JLS 18.5.2.2:
250+
// In this case, the constraints in C that participate in a dependency cycle (or cycles) and do
251+
// not depend on any constraints outside of the cycle (or cycles) are considered.
252+
// A single constraint is selected from the considered constraints, as follows:
253+
254+
// If any of the considered constraints have the form <Expression -> T>, then the selected
255+
// constraint is the considered constraint of this form that contains the expression to the
256+
// left (3.5) of the expression of every other considered constraint of this form.
257+
258+
// If no considered constraint has the form <Expression -> T>, then the selected constraint
259+
// is the considered constraint that contains the expression to the left of the expression
260+
// of every other considered constraint.
197261
List<Constraint> consideredConstraints = new ArrayList<>();
198-
for (Constraint constraint : list) {
262+
for (Constraint constraint : c.list) {
199263
if (!(constraint instanceof TypeConstraint)) {
200264
continue;
201265
}
202-
TypeConstraint c = (TypeConstraint) constraint;
203-
Set<Variable> newInputs = dependencies.get(c.getInputVariables());
204-
Set<Variable> newOutputs = dependencies.get(c.getOutputVariables());
266+
267+
TypeConstraint typeConstraint = (TypeConstraint) constraint;
268+
Set<Variable> newInputs = dependencies.get(typeConstraint.getInputVariables());
269+
Set<Variable> newOutputs = dependencies.get(typeConstraint.getOutputVariables());
205270
if (inputDependencies.isEmpty()
206271
|| !Collections.disjoint(newInputs, outDependencies)
207272
|| !Collections.disjoint(newOutputs, inputDependencies)) {
208273
inputDependencies.addAll(newInputs);
209274
outDependencies.addAll(newOutputs);
210-
consideredConstraints.add(c);
275+
consideredConstraints.add(typeConstraint);
211276
}
212277
}
213278

214-
// A single constraint is selected from the considered constraints, as follows:
215-
216-
// If any of the considered constraints have the form <Expression -> T>, then the selected
217-
// constraint is the considered constraint of this form that contains the expression to the
218-
// left (3.5) of the expression of every other considered constraint of this form.
219-
220-
// If no considered constraint has the form <Expression -> T>, then the selected constraint
221-
// is the considered constraint that contains the expression to the left of the expression
222-
// of every other considered constraint.
223-
224-
for (Constraint c : consideredConstraints) {
225-
if (c.getKind() == Kind.EXPRESSION) {
226-
return new ConstraintSet(c);
279+
for (Constraint constraint : consideredConstraints) {
280+
if (constraint.getKind() == Kind.EXPRESSION) {
281+
return new ConstraintSet(constraint);
227282
}
228283
}
229284

@@ -334,9 +389,19 @@ public BoundSet reduceOneStep(Java8InferenceContext context) {
334389
}
335390
this.addAll(((ReductionResultPair) result).constraintSet);
336391
} else if (result instanceof TypeConstraint) {
337-
this.add((Constraint) result);
392+
// Add the new constraints to the beginning of the list so they are reduced first. This is
393+
// because each constraint is supposed to be reduced until no other constraints are produced
394+
// before moving onto another constraint.
395+
this.push((Constraint) result);
338396
} else if (result instanceof ConstraintSet) {
339-
this.addAll((ConstraintSet) result);
397+
if (result == TRUE_ANNO_FAIL) {
398+
this.annotationFailure = true;
399+
} else {
400+
// Add the new constraints to the beginning of the list so they are reduced first. This is
401+
// because each constraint is supposed to be reduced until no other constraints are produced
402+
// before moving onto another constraint.
403+
this.pushAll((ConstraintSet) result);
404+
}
340405
} else if (result instanceof BoundSet) {
341406
boundSet.merge((BoundSet) result);
342407
if (boundSet.containsFalse()) {

0 commit comments

Comments
 (0)