Skip to content

Commit 4493eee

Browse files
gselzergselzer
authored
Merge pull request #139 from scijava/scijava-ops-engine/conversion-more-anys
Use Anys, not wildcards, for converter output types
2 parents 7d93383 + 6fe3990 commit 4493eee

File tree

5 files changed

+69
-18
lines changed

5 files changed

+69
-18
lines changed

‎scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/convert/ConversionMatchingRoutine.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,11 @@ public OpCandidate findMatch(MatchingConditions conditions, OpMatcher matcher,
7777
.hints()))
7878
{
7979
Conversions.tryConvert(env, info, request).ifPresent(converted -> {
80-
Map<TypeVariable<?>, Type> map = new HashMap<>();
81-
GenericAssignability.inferTypeVariables( //
82-
new Type[] { converted.opType() }, //
83-
new Type[] { request.getType() }, //
84-
map //
85-
);
8680
candidates.add(new OpCandidate( //
8781
env, //
8882
request, //
8983
converted, //
90-
map //
84+
converted.typeVarAssigns() //
9185
));
9286
});
9387
}

‎scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/convert/Conversions.java‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.scijava.function.Mutable;
3535
import org.scijava.ops.api.*;
3636
import org.scijava.ops.engine.BaseOpHints;
37+
import org.scijava.types.Any;
3738
import org.scijava.types.Nil;
3839
import org.scijava.types.Types;
3940
import org.scijava.types.inference.FunctionalInterfaces;
@@ -136,7 +137,7 @@ private static ConvertedOpInfo convert(OpEnvironment env, OpInfo info,
136137
return opt.get();
137138
}
138139
// Attempt 2: Computer with identity mutable output
139-
opt = postprocessIdentity(info, request, preConverters, env);
140+
opt = postprocessIdentity(info, request, preConverters, vars, env);
140141
if (opt.isPresent()) {
141142
return opt.get();
142143
}
@@ -209,7 +210,8 @@ private static Optional<ConvertedOpInfo> postprocessFunction(OpInfo info,
209210
postConverter, //
210211
request.getOutType(), //
211212
null, //
212-
env //
213+
env, //
214+
vars //
213215
));
214216
}
215217

@@ -231,7 +233,7 @@ private static Optional<ConvertedOpInfo> postprocessFunction(OpInfo info,
231233
*/
232234
private static Optional<ConvertedOpInfo> postprocessIdentity(OpInfo info,
233235
OpRequest request, List<RichOp<Function<?, ?>>> preConverters,
234-
OpEnvironment env)
236+
Map<TypeVariable<?>, Type> vars, OpEnvironment env)
235237
{
236238
// This procedure only applies to Ops with mutable outputs
237239
int ioIndex = mutableIndexOf(request.getType());
@@ -252,7 +254,8 @@ private static Optional<ConvertedOpInfo> postprocessIdentity(OpInfo info,
252254
null, //
253255
request.getOutType(), //
254256
null, //
255-
env //
257+
env, //
258+
vars //
256259
));
257260
}
258261
return Optional.empty();
@@ -312,7 +315,8 @@ private static Optional<ConvertedOpInfo> postprocessConvertAndCopy(
312315
postConverter, //
313316
request.getOutType(), //
314317
copyOp, //
315-
env //
318+
env, //
319+
vars //
316320
));
317321
}
318322
catch (OpMatchingException e) {
@@ -375,7 +379,8 @@ private static Optional<ConvertedOpInfo> postprocessCopy(OpInfo info,
375379
postConverter, //
376380
request.getOutType(), //
377381
copyOp, //
378-
env //
382+
env, //
383+
vars //
379384
));
380385
}
381386
catch (OpMatchingException e) {
@@ -443,13 +448,17 @@ private static void resolveTypes(Type source, Type dest,
443448
*/
444449
private static Nil<?> wildcardVacuousTypeVars(final Type t) {
445450
Type[] typeParams = Types.typeParamsAgainstClass(t, Types.raw(t));
451+
if (t instanceof TypeVariable<?>) {
452+
TypeVariable<?> tv = (TypeVariable<?>) t;
453+
return Nil.of(new Any(tv.getBounds()));
454+
}
446455
var vars = new HashMap<TypeVariable<?>, Type>();
447456
for (Type typeParam : typeParams) {
448457
if (typeParam instanceof TypeVariable<?>) {
449458
// Get the type variable
450459
TypeVariable<?> from = (TypeVariable<?>) typeParam;
451460
// Create a wildcard type with the type variable bounds
452-
Type to = Types.wildcard(from.getBounds(), null);
461+
Type to = new Any(from.getBounds());
453462
vars.put(from, to);
454463
}
455464
}

‎scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/convert/ConvertedOpInfo.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ public class ConvertedOpInfo implements OpInfo {
9494

9595
private final OpInfo info;
9696
private final OpEnvironment env;
97+
private final Map<TypeVariable<?>, Type> typeVarAssigns;
9798
final List<RichOp<Function<?, ?>>> preconverters;
9899
final List<Type> inTypes;
99100
final RichOp<Function<?, ?>> postconverter;
@@ -116,7 +117,8 @@ public ConvertedOpInfo(OpInfo info,
116117
Arrays.asList(inTypes(info.inputTypes(), preconverters)), //
117118
postconverter, //
118119
outType(info.outputType(), postconverter), copyOp, //
119-
env //
120+
env, //
121+
Collections.emptyMap() //
120122
);
121123
}
122124

@@ -138,7 +140,8 @@ public ConvertedOpInfo( //
138140
RichOp<Function<?, ?>> postconverter, //
139141
Type reqOutput, //
140142
final RichOp<Computers.Arity1<?, ?>> copyOp, //
141-
OpEnvironment env //
143+
OpEnvironment env, //
144+
Map<TypeVariable<?>, Type> typeVarAssigns //
142145
) {
143146
this.info = info;
144147
this.opType = mapAnys(opType, info);
@@ -153,6 +156,7 @@ public ConvertedOpInfo( //
153156
BaseOpHints.Conversion.FORBIDDEN, //
154157
"converted" //
155158
);
159+
this.typeVarAssigns = typeVarAssigns;
156160
}
157161

158162
/**
@@ -943,4 +947,7 @@ private static String fMethodPreprocessing(
943947
return sb.toString();
944948
}
945949

950+
public Map<TypeVariable<?>, Type> typeVarAssigns() {
951+
return this.typeVarAssigns;
952+
}
946953
}

‎scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/convert/IdentityCollection.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
* @author Gabriel Selzer
4646
* @param <T>
4747
*/
48-
public class IdentityCollection<T, U extends T> implements OpCollection {
48+
public class IdentityCollection<T> implements OpCollection {
4949

5050
/**
5151
* @input t the object to be converted
@@ -55,7 +55,7 @@ public class IdentityCollection<T, U extends T> implements OpCollection {
5555
@OpHints(hints = { Conversion.FORBIDDEN,
5656
BaseOpHints.DependencyMatching.FORBIDDEN })
5757
@OpField(names = "engine.convert, engine.identity", priority = Priority.FIRST)
58-
public final Function<U, T> identity = (t) -> t;
58+
public final Function<T, T> identity = (t) -> t;
5959

6060
/**
6161
* @mutable t the object to be "mutated"

‎scijava-ops-engine/src/test/java/org/scijava/ops/engine/matcher/convert/ConversionTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import static org.junit.jupiter.api.Assertions.assertEquals;
3333

3434
import java.lang.reflect.Type;
35+
import java.lang.reflect.TypeVariable;
3536
import java.util.function.BiFunction;
3637
import java.util.function.Function;
3738

@@ -40,6 +41,7 @@
4041
import org.junit.jupiter.api.Test;
4142
import org.scijava.function.Computers;
4243
import org.scijava.function.Container;
44+
import org.scijava.ops.api.OpInfo;
4345
import org.scijava.ops.api.Ops;
4446
import org.scijava.ops.engine.AbstractTestEnvironment;
4547
import org.scijava.ops.engine.conversionLoss.impl.IdentityLossReporter;
@@ -169,4 +171,43 @@ public void testConvertAnys() {
169171
Assertions.assertInstanceOf(Integer.class, out);
170172
}
171173

174+
/**
175+
* An Op, written as a method, whose type variable has multiple bounds.
176+
* <p>
177+
* Note that, for the purposes of this test, the {@link Number} bound is
178+
* necessary even though it is not needed for the functionality of the test
179+
* Op.
180+
* </p>
181+
*/
182+
@OpMethod(names = "test.boundsConversion", type = BiFunction.class)
183+
public static <T extends Number & Comparable<T>> T foo(T in1, T in2) {
184+
return in1.compareTo(in2) > 0 ? in1 : in2;
185+
}
186+
187+
/**
188+
* Tests that conversion is possible when {@link TypeVariable}s in the
189+
* {@link OpInfo} are bounded by multiple types.
190+
*/
191+
@Test
192+
public void testConvertMultipleBounds() {
193+
// Assert that there's only one possible match for our Op call
194+
var name = "test.boundsConversion";
195+
var infos = ops.infos(name);
196+
Assertions.assertEquals(1, infos.size());
197+
// And its input types are TypeVariables with two upper bounds.
198+
var inType = infos.first().inputTypes().get(0);
199+
Assertions.assertInstanceOf(TypeVariable.class, inType);
200+
var numBounds = ((TypeVariable<?>) inType).getBounds().length;
201+
Assertions.assertEquals(2, numBounds);
202+
// Now, call it such that we need conversion
203+
Integer i1 = 1;
204+
Double i2 = 2.0;
205+
var result = ops.op("test.boundsConversion") //
206+
.arity2().input(i1, i2).apply();
207+
// Assert the result is an Integer
208+
Assertions.assertInstanceOf(Integer.class, result);
209+
Integer intResult = (Integer) result;
210+
Assertions.assertEquals(2, intResult);
211+
}
212+
172213
}

0 commit comments

Comments
 (0)