Gizmo2: add MapOps #253
Gizmo2: add MapOps #253
Conversation
mkouba
commented
Mar 24, 2025
- also add some tests for conditionals
| @@ -784,7 +785,7 @@ private If doIfInsn(final ClassDesc type, final Expr cond, final BlockCreatorImp | |||
| } | |||
| // failed | |||
| } | |||
| return addItem(new IfZero(type, If.Kind.NE, wt, null, (Item) cond)); | |||
| return addItem(new IfZero(type, If.Kind.NE, wt, wf, (Item) cond)); | |||
| int idx = 0; | ||
| Expr args = newEmptyArray(Util.classDesc(Entry.class), Constant.of(size)); | ||
| for (Expr item : items) { | ||
| set(args.elem(idx++), item); | ||
| } | ||
| return invokeStatic(MethodDesc.of(Map.class, "ofEntries", Map.class, Entry[].class), newArray(Object.class, args)); |
There was a problem hiding this comment.
This part doesn't look right, the new array will be the wrong size and it will not contain map entries as is required by the ofEntries method.
This is actually a very tricky thing because each entry has to be constructed before the next entry expressions appear on the instruction list; i.e. calls to Map.entry(k, v) have to be stitched into the list after the fact. It is doable (this is similar to how newArray(Expr...) is implemented) but the algorithm is pretty complex.
Maybe it'd be best to throw an UnsupportedOperationException for maps with more than 10 entries for now, and we can revisit it in a second pass.
There was a problem hiding this comment.
Good catch! I actually had a TODO comment in my local branch but forgot to revisit this code. BTW this part is not tested at all ;-).
Maybe it'd be best to throw an UnsupportedOperationException for maps with more than 10 entries for now, and we can revisit it in a second pass.
+1
| mc.body(bc -> { | ||
| var len = bc.define("len", | ||
| bc.invokeVirtual(MethodDesc.of(String.class, "length", int.class), val)); | ||
| bc.unless(bc.eq(len, 5), bc1 -> bc1.returnFalse()); |
There was a problem hiding this comment.
One reason I added returnTrue and returnFalse is so that you could pass in BlockCreator::returnFalse to these kinds of methods instead of writing the lambda. I guess it's not really less typing, but maybe slightly better for implementation.
- also add some tests for conditionals