Skip to content

Commit 08c075d

Browse files
gselzergselzer
committed
Improve ConvertedOpInfo types
1 parent b30ad2a commit 08c075d

File tree

3 files changed

+124
-40
lines changed

3 files changed

+124
-40
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ private Conversions() {
7676
* @return the index of the mutable argument (or -1 iff the output is
7777
* returned).
7878
*/
79-
public static int findMutableArgIndex(Class<?> c) {
79+
public static int mutableIndexOf(Class<?> c) {
8080
Method fMethod = FunctionalInterfaces.functionalMethodOf(c);
8181
for (int i = 0; i < fMethod.getParameterCount(); i++) {
8282
if (AnnotationUtils.getMethodParameterAnnotation(fMethod, i,
@@ -479,7 +479,7 @@ private static Nil<?> wildcardVacuousTypeVars(final Type t) {
479479
OpInfo info, //
480480
OpRequest request, Hints hints //
481481
) {
482-
int ioIndex = Conversions.findMutableArgIndex(Types.raw(info.opType()));
482+
int ioIndex = Conversions.mutableIndexOf(Types.raw(info.opType()));
483483
// If IO index is -1, output is returned - no need to copy.
484484
if (ioIndex == -1) {
485485
return null;

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

Lines changed: 89 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -120,30 +120,68 @@ private static Type generateOpType(OpInfo info,
120120
List<RichOp<Function<?, ?>>> preconverter,
121121
RichOp<Function<?, ?>> postconverter)
122122
{
123-
Type[] inTypes = inTypes(info.inputTypes().toArray(Type[]::new),
124-
preconverter);
123+
Type[] inTypes = inTypes(info.inputTypes(), preconverter);
125124
Type outType = outType(info.outputType(), postconverter);
126125
Class<?> fIface = FunctionalInterfaces.findFrom(info.opType());
127126
return retypeOpType(fIface, inTypes, outType);
128127
}
129128

130-
public ConvertedOpInfo(OpInfo info, Type opType,
131-
List<RichOp<Function<?, ?>>> preconverters,
132-
RichOp<Function<?, ?>> postconverter,
133-
final RichOp<Computers.Arity1<?, ?>> copyOp, OpEnvironment env)
134-
{
129+
public ConvertedOpInfo( //
130+
OpInfo info, //
131+
Type opType, //
132+
List<RichOp<Function<?, ?>>> preconverters, //
133+
RichOp<Function<?, ?>> postconverter, //
134+
final RichOp<Computers.Arity1<?, ?>> copyOp, //
135+
OpEnvironment env //
136+
) {
135137
this.info = info;
136138
this.opType = opType;
137139
this.preconverters = preconverters;
138140
this.postconverter = postconverter;
139141
this.copyOp = copyOp;
140142
this.env = env;
143+
this.struct = generateStruct( //
144+
info, //
145+
opType, //
146+
preconverters, //
147+
postconverter //
148+
);
149+
this.hints = info.declaredHints().plus( //
150+
BaseOpHints.Conversion.FORBIDDEN, //
151+
"converted" //
152+
);
153+
}
141154

142-
Type[] inTypes = inTypes(info.inputTypes().toArray(Type[]::new),
143-
preconverters);
155+
/**
156+
* Helper method to generate the new {@link Struct}
157+
*
158+
* @param info the original {@link OpInfo}
159+
* @param opType the new Op's {@link Type}
160+
* @param preconverters the {@link Function}s responsible for converting Op
161+
* parameters
162+
* @param postconverter the {@link Function} responsible for converting the
163+
* Op's return
164+
* @return the {@link Struct} of the converted Op
165+
*/
166+
private static Struct generateStruct(OpInfo info, Type opType,
167+
List<RichOp<Function<?, ?>>> preconverters,
168+
RichOp<Function<?, ?>> postconverter)
169+
{
170+
List<Type> originalIns = new ArrayList<>(info.inputTypes());
171+
List<Member<?>> ioMembers = new ArrayList<>(info.struct().members());
172+
// If the mutable index differs between the declared Op type and the
173+
// requested Op type, we must move the IO memberr
174+
int fromIOIdx = Conversions.mutableIndexOf(Types.raw(info.opType()));
175+
int toIOIdx = Conversions.mutableIndexOf(Types.raw(opType));
176+
if (fromIOIdx != toIOIdx) {
177+
originalIns.add(toIOIdx, originalIns.remove(fromIOIdx));
178+
ioMembers.add(toIOIdx, ioMembers.remove(fromIOIdx));
179+
}
180+
// Determine the new input and output types of the Op
181+
Type[] inTypes = inTypes(originalIns, preconverters);
144182
Type outType = outType(info.outputType(), postconverter);
145183

146-
List<Member<?>> ioMembers = info.struct().members();
184+
// Create the functional member types of the new OpInfo
147185
int index = 0;
148186
List<FunctionalMethodType> fmts = new ArrayList<>();
149187
for (Member<?> m : ioMembers) {
@@ -152,12 +190,9 @@ public ConvertedOpInfo(OpInfo info, Type opType,
152190
: null;
153191
fmts.add(new FunctionalMethodType(newType, m.getIOType()));
154192
}
155-
// generate new output fmt
193+
// generate new struct
156194
RetypingRequest r = new RetypingRequest(info.struct(), fmts);
157-
this.struct = Structs.from(r, opType, new OpRetypingMemberParser());
158-
159-
this.hints = info.declaredHints().plus(BaseOpHints.Conversion.FORBIDDEN,
160-
"converted");
195+
return Structs.from(r, opType, new OpRetypingMemberParser());
161196
}
162197

163198
public OpInfo srcInfo() {
@@ -310,28 +345,32 @@ public String id() {
310345
return sb.toString();
311346
}
312347

313-
private static Type[] inTypes(Type[] originalInputs,
314-
List<RichOp<Function<?, ?>>> inputFocusers)
348+
private static Type[] inTypes(List<Type> originalInputs,
349+
List<RichOp<Function<?, ?>>> preconverters)
315350
{
316351
Map<TypeVariable<?>, Type> typeAssigns = new HashMap<>();
317-
Type[] inTypes = new Type[originalInputs.length];
318-
// NB: This feels kind of inefficient, but we have to do each individually
319-
// to avoid the case that the same Op is being used for different types.
320-
// Here's one use edge case - suppose the Op Identity<T> maps A, B, and C.
321-
// The new inTypes should thus be A, B, and C, but if we do them all
322-
// together we'll get inTypes Object from the output of the focusers, as
323-
// there's only one type variable to map across the three inputs.
324-
for (int i = 0; i < originalInputs.length; i++) {
352+
Type[] inTypes = new Type[originalInputs.size()];
353+
for (int i = 0; i < originalInputs.size(); i++) {
325354
typeAssigns.clear();
326-
if (inputFocusers.get(i) == null) {
327-
inTypes[i] = originalInputs[i];
328-
}
329-
else {
330-
var info = Ops.info(inputFocusers.get(i));
331-
GenericAssignability.inferTypeVariables(new Type[] { info
332-
.outputType() }, new Type[] { originalInputs[i] }, typeAssigns);
333-
inTypes[i] = Types.mapVarToTypes(info.inputTypes().get(0), typeAssigns);
355+
// Start by looking at the input type T of the preconverter
356+
var type = preconverters.get(i).instance().getType();
357+
var pType = (ParameterizedType) type;
358+
inTypes[i] = pType.getActualTypeArguments()[0];
359+
// Sometimes, the type of the Op instance can contain wildcards.
360+
// These do not help us in determining the new type, so we have to
361+
// start over with the input converter input type.
362+
if (inTypes[i] instanceof WildcardType) {
363+
inTypes[i] = Ops.info(preconverters.get(i)).inputTypes().get(0);
334364
}
365+
// Infer type variables in the preconverter input w.r.t. the
366+
// parameter types of the ORIGINAL OpInfo
367+
GenericAssignability.inferTypeVariables( //
368+
new Type[] { pType.getActualTypeArguments()[1] }, //
369+
new Type[] { originalInputs.get(i) }, //
370+
typeAssigns //
371+
);
372+
// Map type variables in T to Types inferred above
373+
inTypes[i] = Types.mapVarToTypes(inTypes[i], typeAssigns);
335374
}
336375
return inTypes;
337376
}
@@ -340,14 +379,26 @@ private static Type outType( //
340379
Type originalOutput, //
341380
RichOp<Function<?, ?>> postconverter //
342381
) {
343-
Map<TypeVariable<?>, Type> typeAssigns = new HashMap<>();
382+
// Start by looking at the output type T of the postconverter
383+
var type = postconverter.instance().getType();
384+
var pType = (ParameterizedType) type;
385+
Type outType = pType.getActualTypeArguments()[1];
386+
// Sometimes, the type of the Op instance can contain wildcards.
387+
// These do not help us in determining the new type, so we have to
388+
// start over with the postconverter's output type.
389+
if (outType instanceof WildcardType) {
390+
outType = Ops.info(postconverter).outputType();
391+
}
392+
Map<TypeVariable<?>, Type> vars = new HashMap<>();
393+
// Infer type variables in the postconverter input w.r.t. the
394+
// parameter types of the ORIGINAL OpInfo
344395
GenericAssignability.inferTypeVariables( //
345-
new Type[] { Ops.info(postconverter).inputTypes().get(0) }, //
396+
new Type[] { pType.getActualTypeArguments()[0] }, //
346397
new Type[] { originalOutput }, //
347-
typeAssigns //
398+
vars //
348399
);
349-
return Types.mapVarToTypes(Ops.info(postconverter).outputType(),
350-
typeAssigns);
400+
// map type variables in T to Types inferred in Step 2a
401+
return Types.mapVarToTypes(outType, vars);
351402
}
352403

353404
@Override

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,15 @@
3131

3232
import static org.junit.jupiter.api.Assertions.assertEquals;
3333

34+
import java.lang.reflect.Type;
3435
import java.util.function.BiFunction;
3536

37+
import org.junit.jupiter.api.Assertions;
3638
import org.junit.jupiter.api.BeforeAll;
3739
import org.junit.jupiter.api.Test;
40+
import org.scijava.function.Computers;
41+
import org.scijava.function.Container;
42+
import org.scijava.ops.api.Ops;
3843
import org.scijava.ops.engine.AbstractTestEnvironment;
3944
import org.scijava.ops.engine.conversionLoss.impl.IdentityLossReporter;
4045
import org.scijava.ops.engine.conversionLoss.impl.PrimitiveLossReporters;
@@ -43,6 +48,7 @@
4348
import org.scijava.ops.engine.matcher.impl.LossReporterWrapper;
4449
import org.scijava.ops.spi.OpCollection;
4550
import org.scijava.ops.spi.OpField;
51+
import org.scijava.ops.spi.OpMethod;
4652

4753
/**
4854
* Basic Op conversion test
@@ -117,4 +123,31 @@ public void testConvertedOp() {
117123
assertEquals(81., result, 0);
118124
}
119125

126+
@OpMethod(names = "test.differentOrder", type = Computers.Arity1_1.class)
127+
public static void foo(@Container Integer[] i1, Double[] i2) {
128+
for (int i = 0; i < i1.length; i++) {
129+
i1[i] = i2[i].intValue() * i2[i].intValue();
130+
}
131+
}
132+
133+
@Test
134+
public void testConvertedOpInfo() {
135+
Long[] i1 = { 1L };
136+
Byte[] i2 = { 2 };
137+
var op = ops.op("test.differentOrder").arity1().input(i1).output(i2)
138+
.computer();
139+
var info = Ops.rich(op).infoTree().info();
140+
Assertions.assertInstanceOf(ConvertedOpInfo.class, info);
141+
// Assert input types are expected
142+
Type[] expected = { Long[].class, Byte[].class };
143+
Assertions.assertArrayEquals(expected, info.inputTypes().toArray());
144+
// Assert the output type is expected
145+
Assertions.assertEquals(Byte[].class, info.outputType());
146+
147+
// Assert the struct types are correct
148+
var members = info.struct().members();
149+
Assertions.assertEquals(Long[].class, members.get(0).getType());
150+
Assertions.assertEquals(Byte[].class, members.get(1).getType());
151+
}
152+
120153
}

0 commit comments

Comments
 (0)