Skip to content

Commit 2830902

Browse files
Treiblesschorlectrueden
Treiblesschorle
authored and
ctrueden
committed
Use generic assignability test to determine if candidate matches OpRef types instead of satisfiability
1 parent 9d543fd commit 2830902

8 files changed

Lines changed: 141 additions & 114 deletions

File tree

‎src/main/java/org/scijava/ops/base/DefaultOpTypeMatchingService.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public List<OpCandidate> findCandidates(final OpEnvironment ops, final List<OpRe
8989
final ArrayList<OpCandidate> candidates = new ArrayList<>();
9090
for (final OpInfo info : ops.infos()) {
9191
for (final OpRef ref : refs) {
92-
if (satisfiesRefTypes(info, ref)) {
92+
if (ref.typesMatch(info.opClass())) {
9393
candidates.add(new OpCandidate(ops, ref, info));
9494
}
9595
}
@@ -148,7 +148,7 @@ public boolean typesMatch(final OpCandidate candidate) {
148148
return false;
149149
}
150150

151-
int conflictingIndex = Types.satisfies(candidateArgTypes, refArgTypes);
151+
int conflictingIndex = Types.satisfies(refArgTypes, candidateArgTypes);
152152
if (conflictingIndex != -1) {
153153
final Type to = refArgTypes[conflictingIndex];
154154
final Type from = candidateArgTypes[conflictingIndex];
@@ -162,17 +162,6 @@ public boolean typesMatch(final OpCandidate candidate) {
162162

163163
// -- Helper methods --
164164

165-
/**
166-
* Checks whether the op class satisfies the types of the specified {@link OpRef}.
167-
*
168-
* @param info the info to get the op class from
169-
* @param ref the ref to get the types from
170-
* @return
171-
*/
172-
private boolean satisfiesRefTypes(final OpInfo info, final OpRef ref) {
173-
return ref.typeSatisfies(info.opClass());
174-
}
175-
176165
/**
177166
* Performs several checks, whether the specified candidate:</br></br>
178167
* * {@link #isValid(OpCandidate)}</br>

‎src/main/java/org/scijava/ops/base/OpRef.java‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@
2929

3030
package org.scijava.ops.base;
3131

32+
import java.lang.reflect.ParameterizedType;
3233
import java.lang.reflect.Type;
3334
import java.util.Arrays;
3435
import java.util.Objects;
3536

3637
import org.scijava.ops.Op;
38+
import org.scijava.util.TypeUtils;
3739
import org.scijava.util.Types;
3840

3941
/**
@@ -131,36 +133,28 @@ public String getLabel() {
131133
* Determines whether the specified type satisfies the op's required types
132134
* using {@link Types#satisfies(Type[], Type[])}.
133135
*/
134-
public boolean typeSatisfies(final Type type) {
136+
public boolean typesMatch(final Class<?> opClass) {
135137
if (types == null)
136138
return true;
137-
Type[] repeat = new Type[types.length];
138-
Arrays.fill(repeat, type);
139-
return Types.satisfies(repeat, types) == -1;
139+
for (Type t : types) {
140+
if(t instanceof ParameterizedType) {
141+
if (!TypeUtils.checkGenericAssignability(opClass, (ParameterizedType) t)) {
142+
return false;
143+
}
144+
} else {
145+
if (!Types.isAssignable(opClass, t)) {
146+
return false;
147+
}
148+
}
149+
}
150+
return true;
140151
}
141152

142153
// -- Object methods --
143154

144155
@Override
145156
public String toString() {
146157
return Arrays.deepToString(types);
147-
// StringBuilder sb = new StringBuilder();
148-
// sb.append(getLabel());
149-
// sb.append("(");
150-
// boolean first = true;
151-
// for (Object arg : args) {
152-
// if (first) first = false;
153-
// else sb.append(", ");
154-
// if (arg.getClass() == Class.class) {
155-
// // special typed null placeholder
156-
// sb.append(((Class<?>) arg).getSimpleName());
157-
// }
158-
// else sb.append(arg.getClass().getSimpleName());
159-
//
160-
// }
161-
// sb.append(")");
162-
//
163-
// return sb.toString();
164158
}
165159

166160
@Override

‎src/main/java/org/scijava/ops/math/Add.java‎

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -48,42 +48,21 @@ public double[] apply(double[] arr1, double[] arr2) {
4848
return Streams.zip(s1, s2, (num1, num2) -> num1 + num2).mapToDouble(Double::doubleValue).toArray();
4949
}
5050
}
51-
51+
5252
@Plugin(type = MathAddOp.class, priority = Priority.HIGH)
5353
@Parameter(key = "iter1")
5454
@Parameter(key = "iter2")
5555
@Parameter(key = "resultArray", type = ItemIO.OUTPUT)
56-
public static class MathPointwiseAddIterableDoublesFunction
57-
implements MathAddOp, BiFunction<Iterable<Double>, Iterable<Double>, Iterable<Double>> {
58-
56+
public static class MathPointwiseAddIterablesFunction<M extends Number, I extends Iterable<M>>
57+
implements MathAddOp, BiFunction<I, I, Iterable<Double>> {
5958
@Override
60-
public Iterable<Double> apply(Iterable<Double> i1, Iterable<Double> i2) {
61-
Stream<Double> s1 = Streams.stream(i1);
62-
Stream<Double> s2 = Streams.stream(i2);
63-
return () -> Streams.zip(s1, s2, (e1, e2) -> e1 + e2).iterator();
59+
public Iterable<Double> apply(I i1, I i2) {
60+
Stream<? extends Number> s1 = Streams.stream((Iterable<? extends Number>) i1);
61+
Stream<? extends Number> s2 = Streams.stream((Iterable<? extends Number>) i2);
62+
return () -> Streams.zip(s1, s2, (e1, e2) -> e1.doubleValue() + e2.doubleValue()).iterator();
6463
}
65-
6664
}
6765

68-
// Do we want to have generic types ops?
69-
// Would be cool if someone is looking for an op that expects double and we give him the generic op which works on all numbers.
70-
// Currently, the op calss itself can't have type arguments (possible bug in Types.satisfies) however it works with wildcards
71-
// when there is only an upper bound needed. See below.
72-
73-
// @Plugin(type = MathAddOp.class, priority = Priority.HIGH)
74-
// @Parameter(key = "iter1")
75-
// @Parameter(key = "iter2")
76-
// @Parameter(key = "resultArray", type = ItemIO.OUTPUT)
77-
// public static class MathPointwiseAddIterablesFunction
78-
// implements MathAddOp, BiFunction<Iterable<? extends Number>, Iterable<? extends Number>, Iterable<Double>> {
79-
// @Override
80-
// public Iterable<Double> apply(Iterable<? extends Number> i1, Iterable<? extends Number> i2) {
81-
// Stream<? extends Number> s1 = Streams.stream(i1);
82-
// Stream<? extends Number> s2 = Streams.stream(i2);
83-
// return () -> Streams.zip(s1, s2, (e1, e2) -> e1.doubleValue() + e2.doubleValue()).iterator();
84-
// }
85-
// }
86-
8766
// --------- Computers ---------
8867

8968
@Plugin(type = MathAddOp.class)

‎src/main/java/org/scijava/ops/util/Computers.java‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package org.scijava.ops.util;
22

3+
import java.lang.reflect.Type;
4+
import java.util.function.Function;
5+
36
import org.scijava.ops.BiComputer;
47
import org.scijava.ops.Computer;
58
import org.scijava.ops.Op;
69
import org.scijava.ops.base.OpService;
710
import org.scijava.ops.types.Nil;
11+
import org.scijava.util.Types;
812

913
/**
1014
* Utility providing adaptation between {@link Op} types.
@@ -17,22 +21,37 @@ private Computers() {
1721

1822
public static <I, O> Computer<I, O> unary(final OpService ops, final Class<? extends Op> opClass,
1923
final Nil<I> inputType, final Nil<O> outputType, final Object... secondaryArgs) {
24+
25+
Nil<Computer<I, O>> computerNil = new Nil<Computer<I, O>>() {
26+
@Override
27+
public Type getType() {
28+
return Types.parameterize(Computer.class, new Type[] { inputType.getType(), outputType.getType() });
29+
}
30+
};
31+
2032
return ops.findOp( //
2133
opClass, //
22-
new Nil<Computer<I, O>>() {
23-
}, //
34+
computerNil, //
2435
new Nil[] { inputType, outputType }, //
25-
new Nil[] { outputType }, //
36+
new Nil[] { outputType }, //
2637
secondaryArgs);
2738
}
2839

2940
public static <I1, I2, O> BiComputer<I1, I2, O> binary(final OpService ops, final Class<? extends Op> opClass,
3041
final Nil<I1> input1Type, final Nil<I2> input2Type, final Nil<O> outputType,
3142
final Object... secondaryArgs) {
43+
44+
Nil<BiComputer<I1, I2, O>> computerNil = new Nil<BiComputer<I1, I2, O>>() {
45+
@Override
46+
public Type getType() {
47+
return Types.parameterize(BiComputer.class,
48+
new Type[] { input1Type.getType(), input2Type.getType(), outputType.getType() });
49+
}
50+
};
51+
3252
return ops.findOp( //
3353
opClass, //
34-
new Nil<BiComputer<I1, I2, O>>() {
35-
}, //
54+
computerNil, //
3655
new Nil[] { input1Type, input2Type, outputType }, //
3756
new Nil[] { outputType }, //
3857
secondaryArgs);

‎src/main/java/org/scijava/ops/util/Functions.java‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package org.scijava.ops.util;
22

3+
import java.lang.reflect.ParameterizedType;
4+
import java.lang.reflect.Type;
35
import java.util.function.BiFunction;
46
import java.util.function.Function;
57

68
import org.scijava.ops.Op;
79
import org.scijava.ops.base.OpService;
810
import org.scijava.ops.types.Nil;
11+
import org.scijava.util.Types;
912

1013
/**
1114
* Utility providing adaptation between {@link Op} types.
@@ -18,10 +21,17 @@ private Functions() {
1821

1922
public static <I, O> Function<I, O> unary(final OpService ops, final Class<? extends Op> opClass,
2023
final Nil<I> inputType, final Nil<O> outputType, final Object... secondaryArgs) {
24+
25+
Nil<Function<I, O>> functionNil = new Nil<Function<I, O>>() {
26+
@Override
27+
public Type getType() {
28+
return Types.parameterize(Function.class, new Type[] { inputType.getType(), outputType.getType() });
29+
}
30+
};
31+
2132
return ops.findOp( //
2233
opClass, //
23-
new Nil<Function<I, O>>() {
24-
}, //
34+
functionNil, //
2535
new Nil[] { inputType }, //
2636
new Nil[] { outputType }, //
2737
secondaryArgs);
@@ -31,18 +41,17 @@ public static <I1, I2, O> BiFunction<I1, I2, O> binary(final OpService ops, fina
3141
final Nil<I1> input1Type, final Nil<I2> input2Type, final Nil<O> outputType,
3242
final Object... secondaryArgs) {
3343

34-
// Parameterize special type corresponding to this method with in/out types. Then we do not have to specify them
35-
// explicitly anymore? Also, outside of this 'Functions' convenience thing, one could just search for the deeply
36-
// typed (having all type variables bound to a specific type) special type. As we are using Types.satisfies, this
37-
// should be type safe. Hence, we could allow to not specify the ins/outs if the special type does not contain
38-
// type variables.
39-
// ParameterizedType parameterizedBiFuncType = Types.parameterize(BiFunction.class,
40-
// new Type[] { input1Type.getType(), input2Type.getType(), outputType.getType() });
44+
Nil<BiFunction<I1, I2, O>> functionNil = new Nil<BiFunction<I1, I2, O>>() {
45+
@Override
46+
public Type getType() {
47+
return Types.parameterize(BiFunction.class,
48+
new Type[] { input1Type.getType(), input2Type.getType(), outputType.getType() });
49+
}
50+
};
4151

4252
return ops.findOp( //
4353
opClass, //
44-
new Nil<BiFunction<I1, I2, O>>() {
45-
}, //
54+
functionNil, //
4655
new Nil[] { input1Type, input2Type }, //
4756
new Nil[] { outputType }, //
4857
secondaryArgs);

‎src/main/java/org/scijava/ops/util/Inplaces.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package org.scijava.ops.util;
22

3+
import java.lang.reflect.Type;
4+
35
import org.scijava.ops.BiInplace1;
6+
import org.scijava.ops.Computer;
47
import org.scijava.ops.Inplace;
58
import org.scijava.ops.Op;
69
import org.scijava.ops.base.OpService;
710
import org.scijava.ops.types.Nil;
11+
import org.scijava.util.Types;
812

913
/**
1014
* Utility providing adaptation between {@link Op} types.
@@ -17,21 +21,36 @@ private Inplaces() {
1721

1822
public static <IO> Inplace<IO> unary(final OpService ops, final Class<? extends Op> opClass,
1923
final Nil<IO> inputOutputType, final Object... secondaryArgs) {
24+
25+
Nil<Inplace<IO>> inplaceNil = new Nil<Inplace<IO>>() {
26+
@Override
27+
public Type getType() {
28+
return Types.parameterize(Inplace.class, new Type[] { inputOutputType.getType() });
29+
}
30+
};
31+
2032
return ops.findOp( //
2133
opClass, //
22-
new Nil<Inplace<IO>>() {
23-
}, //
34+
inplaceNil, //
2435
new Nil[] { inputOutputType }, //
2536
new Nil[] { inputOutputType }, //
2637
secondaryArgs);
2738
}
2839

2940
public static <IO, I2> BiInplace1<IO, I2> binary1(final OpService ops, final Class<? extends Op> opClass,
3041
final Nil<IO> inputOutputType, final Nil<I2> input2Type, final Object... secondaryArgs) {
42+
43+
Nil<BiInplace1<IO, I2>> inplaceNil = new Nil<BiInplace1<IO, I2>>() {
44+
@Override
45+
public Type getType() {
46+
return Types.parameterize(BiInplace1.class,
47+
new Type[] { inputOutputType.getType(), input2Type.getType() });
48+
}
49+
};
50+
3151
return ops.findOp( //
3252
opClass, //
33-
new Nil<BiInplace1<IO, I2>>() {
34-
}, //
53+
inplaceNil, //
3554
new Nil[] { inputOutputType, input2Type }, //
3655
new Nil[] { inputOutputType }, //
3756
secondaryArgs);

‎src/main/java/org/scijava/util/TypeUtils.java‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -221,28 +221,27 @@ private static void inferTypeVariables(Type[] types, Type[] inferFrom, Map<TypeV
221221
throw new TypeInferenceException();
222222
}
223223
}
224-
225-
// Bounds could also contain type vars, hence go into recursion
224+
225+
// Bounds could also contain type vars, hence possibly go into recursion
226226
for (Type bound : varType.getBounds()) {
227-
// If the bound of the current var to infer is also a var:
228-
// If we already encountered the var bound, we check if the current type to infer from is
229-
// assignable to the already inferred bound. In this case we do not require equality as
230-
// one var is bounded by another and not the same.
231-
// E.g. we want to infer thy types of vars:
232-
// A extends Number, B extends A
233-
// From types:
234-
// Number, Double
235-
// First A is bound to Number, next B to Double. Then we check the bounds for B. We encounter A,
236-
// for which we already inferred Number. Hence, it suffices to check whether Double can be assigned
237-
// to Number, it does not have to be equal as it is just a transitive bound for B.
238-
// Else go into recursion as we encountered a ned var.
239-
if (bound instanceof TypeVariable && typeAssigns.get((TypeVariable<?>)bound) != null) {
240-
Type typeAssignForBound = typeAssigns.get((TypeVariable<?>)bound);
241-
if(!Types.isAssignable(from, typeAssignForBound)) {
227+
if (bound instanceof TypeVariable && typeAssigns.get((TypeVariable<?>) bound) != null) {
228+
// If the bound of the current var (let's call it A) to infer is also a var (let's call it B):
229+
// If we already encountered B, we check if the current type to infer from is assignable to
230+
// the already inferred type for B. In this case we do not require equality as one var is
231+
// bounded by another and it is not the same. E.g. assume we want to infer the types of vars:
232+
// A extends Number, B extends A
233+
// From types:
234+
// Number, Double
235+
// First A is bound to Number, next B to Double. Then we check the bounds for B. We encounter A,
236+
// for which we already inferred Number. Hence, it suffices to check whether Double can be assigned
237+
// to Number, it does not have to be equal as it is just a transitive bound for B.
238+
Type typeAssignForBound = typeAssigns.get((TypeVariable<?>) bound);
239+
if (!Types.isAssignable(from, typeAssignForBound)) {
242240
throw new TypeInferenceException();
243241
}
244242
} else {
245-
inferTypeVariables(new Type[]{bound}, new Type[]{from}, typeAssigns);
243+
// Else go into recursion as we encountered a new var.
244+
inferTypeVariables(new Type[] { bound }, new Type[] { from }, typeAssigns);
246245
}
247246
}
248247
} else if (types[i] instanceof ParameterizedType) {

0 commit comments

Comments
 (0)