Skip to content

Commit f716a6d

Browse files
committed
Improve Any bounds checking
The broader goal here is to understand "bounded" Anys, which are very necessary for Computer -> Function adaptation. Suppose we ask for a Function<IntType, Any> , hoping to match some Computers.Arity1<I extends RealType<I>, O extends RealType<O>> via adaptation. Adapation requires (1) the underlying Computers.Arity1<I, O> Op, but also (2) some way to generate the output (in practice, either a Function<IntType, Any> or a Producer<Any>). If there are no bounds on the Any, then we can get incorrect matches, such as a Producer<Double>, which would produce an object unsuitable for the Computers.Arity1 op being adapted. Therefore it is necessary to attach bounds on the Any (as an example, an Any bounded by RealType) to ensure correct matching.
1 parent 62f2078 commit f716a6d

File tree

6 files changed

+104
-26
lines changed

6 files changed

+104
-26
lines changed

scijava-common3/src/main/java/org/scijava/common3/Types.java

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,8 +1127,23 @@ private static boolean isAssignable(final Type type, final Any toType,
11271127
final Map<TypeVariable<?>, Type> typeVarAssigns)
11281128
{
11291129
if (type instanceof TypeVariable) {
1130-
// TODO: do we need to do here what we do with the ParameterizedType?
1130+
// If the type variable has an assignment already, we should check against that instead.
1131+
TypeVariable<?> typeVar = (TypeVariable<?>) type;
1132+
if (typeVarAssigns.containsKey(typeVar)) {{
1133+
return isAssignable(typeVarAssigns.get(typeVar), toType, typeVarAssigns);
1134+
}}
11311135
}
1136+
1137+
// For a type to be assignable to an Any, it must fit within the Any's upper and lower bounds.
1138+
for (var upperBound : toType.getUpperBounds()) {
1139+
if (!isAssignable(type, upperBound)) return false;
1140+
}
1141+
1142+
for (var lowerBound : toType.getLowerBounds()) {
1143+
if (!isAssignable(lowerBound, type)) return false;
1144+
}
1145+
1146+
// Now that we know the type is assignable to an Any, we can do some type variable mapping.
11321147
if (type instanceof ParameterizedType) {
11331148
// check if any of the type parameters are TypeVariables, and if so
11341149
// bind them to a new Any with the bounds of the TypeVariable.
@@ -1145,14 +1160,6 @@ private static boolean isAssignable(final Type type, final Any toType,
11451160
return true;
11461161
}
11471162

1148-
for (var upperBound : toType.getUpperBounds()) {
1149-
if (!isAssignable(type, upperBound)) return false;
1150-
}
1151-
1152-
for (var lowerBound : toType.getLowerBounds()) {
1153-
if (!isAssignable(lowerBound, type)) return false;
1154-
}
1155-
11561163
return true;
11571164
}
11581165

@@ -1367,10 +1374,14 @@ else if (!isAssignable(fromResolved == null ? fromTypeArg
13671374
typeVarAssigns.put(unbounded, fromResolved);
13681375
toResolved = fromResolved;
13691376
}
1370-
// bind unbounded to another type variable
1377+
// bind unbounded to a TypeVariable or Any
13711378
else {
1372-
typeVarAssigns.put((TypeVariable<?>) toTypeVarAssigns.get(var),
1373-
fromTypeVarAssigns.get(var));
1379+
TypeVariable<?> to = (TypeVariable<?>) toTypeVarAssigns.get(var);
1380+
Type from = fromTypeVarAssigns.get(var);
1381+
if (from == null) {
1382+
from = new Any(to.getBounds());
1383+
}
1384+
typeVarAssigns.put(to, from);
13741385
}
13751386
}
13761387

@@ -1379,8 +1390,20 @@ else if (!isAssignable(fromResolved == null ? fromTypeArg
13791390
// parameters of the target type.
13801391
if (fromResolved != null && !fromResolved.equals(toResolved)) {
13811392
// check for anys
1382-
if (Any.is(fromResolved) || Any.is(toResolved)) continue;
1383-
if (fromResolved instanceof ParameterizedType &&
1393+
if (Any.is(toResolved)) {
1394+
Any a = toResolved instanceof Any ? (Any) toResolved : new Any();
1395+
for (Type upper: a.getUpperBounds()) {
1396+
if (!Types.isAssignable(fromResolved, upper))
1397+
return false;
1398+
}
1399+
for (Type lower: a.getLowerBounds()) {
1400+
if (!Types.isAssignable(lower, fromResolved))
1401+
return false;
1402+
}
1403+
continue;
1404+
}
1405+
else if (Any.is(fromResolved)) continue;
1406+
else if (fromResolved instanceof ParameterizedType &&
13841407
toResolved instanceof ParameterizedType)
13851408
{
13861409
if (raw(fromResolved) != raw(toResolved)) {
@@ -1791,8 +1814,8 @@ private static boolean isAssignable(final Type type,
17911814
}
17921815

17931816
if (Any.is(type)) {
1794-
typeVarAssigns.put(toTypeVariable, new Any(toTypeVariable.getBounds()));
1795-
return true;
1817+
typeVarAssigns.putIfAbsent(toTypeVariable, new Any(toTypeVariable.getBounds()));
1818+
return isAssignable(typeVarAssigns.get(toTypeVariable), type);
17961819
}
17971820

17981821
throw new IllegalStateException("found an unhandled type: " + type);

scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/adapt/AdaptationMatchingRoutine.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.scijava.ops.engine.matcher.OpCandidate.StatusCode;
4141
import org.scijava.ops.engine.matcher.OpMatcher;
4242
import org.scijava.ops.engine.matcher.impl.DefaultOpRequest;
43-
import org.scijava.ops.engine.struct.FunctionalMethodType;
4443
import org.scijava.ops.engine.struct.FunctionalParameters;
4544
import org.scijava.ops.engine.util.Infos;
4645
import org.scijava.priority.Priority;
@@ -172,10 +171,11 @@ public OpCandidate findMatch(MatchingConditions conditions, OpMatcher matcher,
172171
* @param candidate the {@link OpCandidate} matched for the adaptor input
173172
* @param map the mapping
174173
*/
175-
private void captureTypeVarsFromCandidate(Type adaptorType, OpCandidate candidate,
176-
Map<TypeVariable<?>, Type> map)
177-
{
178-
174+
private void captureTypeVarsFromCandidate(
175+
Type adaptorType,
176+
OpCandidate candidate,
177+
Map<TypeVariable<?>, Type> map
178+
) {
179179
// STEP 1: Base adaptor type variables
180180
// For example, let's say we're operating on Computer<I, O>s.
181181
// The adaptor might work on Computer<T, T>s. Which we need to resolve.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package org.scijava.types.infer;
2+
3+
import org.scijava.common3.Any;
4+
import org.scijava.common3.Types;
5+
6+
import java.lang.reflect.Type;
7+
import java.lang.reflect.TypeVariable;
8+
9+
/**
10+
* @author Gabriel Selzer
11+
*/
12+
class AnyTypeMapping extends TypeMapping {
13+
14+
private final Any boundingAny;
15+
16+
AnyTypeMapping(TypeVariable<?> typeVar, Type any,
17+
boolean malleable)
18+
{
19+
super(typeVar, any, malleable);
20+
this.boundingAny = any instanceof Any ? (Any) any : new Any(typeVar.getBounds());
21+
}
22+
23+
@Override
24+
public void refine(Type newType, boolean malleable) {
25+
super.refine(newType, malleable);
26+
for(Type t: boundingAny.getLowerBounds()) {
27+
if (!Types.isAssignable(t, newType)) {
28+
throw new TypeInferenceException("The new type " + newType + " is not a parent of the Any bound " + t);
29+
}
30+
}
31+
for(Type t: boundingAny.getUpperBounds()) {
32+
if (!Types.isAssignable(newType, t)) {
33+
throw new TypeInferenceException("The new type " + newType + " is not a child of the Any bound " + t);
34+
}
35+
}
36+
}
37+
}

scijava-types/src/main/java/org/scijava/types/infer/GenericAssignability.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,10 @@ private static TypeMapping suitableTypeMapping(TypeVariable<?> typeVar,
676676
return new WildcardTypeMapping(typeVar, (WildcardType) newType,
677677
malleability);
678678
}
679+
if (Any.is(newType)) {
680+
Any a = newType instanceof Any ? (Any) newType : new Any(typeVar.getBounds());
681+
return new AnyTypeMapping(typeVar, a, malleability);
682+
}
679683
return new TypeMapping(typeVar, newType, malleability);
680684
}
681685

scijava-types/src/main/java/org/scijava/types/infer/TypeMapping.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,20 @@ public void refine(Type otherType, boolean newTypeMalleability) {
8585
return;
8686
}
8787
if (Any.is(otherType)) {
88+
// verify Any's bounds:
89+
if (otherType instanceof Any) {
90+
Any casted = (Any) otherType;
91+
for (Type t: casted.getUpperBounds()) {
92+
if (!Types.isAssignable(mappedType, t)) {
93+
throw new TypeInferenceException("Any " + casted + " has an upper bound on " + t + ", which is outside of the current mapped type " + mappedType + " of TypeVariable " + typeVar);
94+
}
95+
}
96+
for (Type t: casted.getLowerBounds()) {
97+
if (!Types.isAssignable(t, mappedType)) {
98+
throw new TypeInferenceException("Any " + casted + " has an lower bound on " + t + ", which is outside of the current mapped type " + mappedType + " of TypeVariable " + typeVar);
99+
}
100+
}
101+
}
88102
return;
89103
}
90104
if (otherType instanceof WildcardType) {

scijava-types/src/test/java/org/scijava/types/infer/InferTypeVariablesTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public <O extends Number> void testInferOToAny()
205205
// We expect O = Any
206206
TypeVariable<?> typeVarO = (TypeVariable<?>) new Nil<O>() {}.type();
207207
Map<TypeVariable<?>, TypeMapping> expected = new HashMap<>();
208-
expected.put(typeVarO, new TypeMapping(typeVarO, Any.class, true));
208+
expected.put(typeVarO, new TypeMapping(typeVarO, new Any(new Type[]{Number.class}), true));
209209

210210
Assertions.assertEquals(expected, typeAssigns);
211211
}
@@ -223,7 +223,7 @@ public <O extends Number> void testInferOToAnyWithClass()
223223
// We expect O = Any
224224
TypeVariable<?> typeVarO = (TypeVariable<?>) new Nil<O>() {}.type();
225225
Map<TypeVariable<?>, TypeMapping> expected = new HashMap<>();
226-
expected.put(typeVarO, new TypeMapping(typeVarO, Any.class, true));
226+
expected.put(typeVarO, new TypeMapping(typeVarO, new Any(new Type[] {Number.class}), true));
227227

228228
Assertions.assertEquals(expected, typeAssigns);
229229
}
@@ -241,7 +241,7 @@ public <O extends Number> void testInferOToAnyWithInterface()
241241
// We expect O = Any
242242
TypeVariable<?> typeVarO = (TypeVariable<?>) new Nil<O>() {}.type();
243243
Map<TypeVariable<?>, TypeMapping> expected = new HashMap<>();
244-
expected.put(typeVarO, new TypeMapping(typeVarO, Any.class, true));
244+
expected.put(typeVarO, new TypeMapping(typeVarO, new Any(new Type[] {Number.class}), true));
245245

246246
Assertions.assertEquals(expected, typeAssigns);
247247
}
@@ -259,7 +259,7 @@ public <O extends Number> void testInferOToAnyWithRawType()
259259
// We expect O = Any
260260
TypeVariable<?> typeVarO = (TypeVariable<?>) new Nil<O>() {}.type();
261261
Map<TypeVariable<?>, TypeMapping> expected = new HashMap<>();
262-
expected.put(typeVarO, new TypeMapping(typeVarO, Any.class, true));
262+
expected.put(typeVarO, new TypeMapping(typeVarO, new Any(new Type[] {Number.class}), true));
263263

264264
Assertions.assertEquals(expected, typeAssigns);
265265
}
@@ -425,7 +425,7 @@ public <T> void testWildcardTypeInference() throws TypeInferenceException {
425425
paramType }, new java.lang.reflect.Type[] { argType }, typeAssigns);
426426
TypeVariable<?> typeVar = (TypeVariable<?>) new Nil<L>() {}.type();
427427
final Map<TypeVariable<?>, Type> expected = new HashMap<>();
428-
expected.put(typeVar, Any.class);
428+
expected.put(typeVar, new Any(new Type[] {new Nil<RecursiveThing<L>>() {}.type()}));
429429
Assertions.assertEquals(expected, typeAssigns);
430430
}
431431
}

0 commit comments

Comments
 (0)