Skip to content

Commit fc3c43b

Browse files
committed
41.5: Fix try/catch peephole bug + sorted collection support
- Fix peephole optimizer not remapping try_begin jump offset, causing catch handler to be silently bypassed when superinstructions removed instructions from the try body - Fix sorted collections: preserve comparator through transient/persistent cycle, with-meta, and sorted? predicate - Fix into to bypass transient path for sorted collections and use JVM completion pattern for halt-when compatibility - Fix take 0 returning nil instead of empty list - Fix case macro to throw ex-info instead of plain string
1 parent e08ba39 commit fc3c43b

7 files changed

Lines changed: 55 additions & 27 deletions

File tree

.dev/memo.md

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,28 @@ Phase 41: Polish & Hardening
4242
2. [x] 41.2: clojure.edn/read — SKIPPED (needs PushbackReader)
4343
3. [x] 41.3: clojure.core.reducers — SKIPPED (needs reify, protocols, ForkJoin)
4444
4. [x] 41.4: Upstream test porting
45-
5. [ ] 41.5: Additional bug fixes and edge cases
45+
5. [x] 41.5: Additional bug fixes and edge cases
4646

4747
## Current Task
4848

49-
41.5: Continue bug fixes and edge cases (round 3).
50-
- Remaining known issues: control.clj (case throw in VM), sequences (subseq), transducers (interpose transducer+take, halt-when)
49+
Phase 41 complete. Plan next phase.
5150

5251
## Previous Task
5352

54-
41.5: Bug fixes and edge cases (round 2).
55-
- Fixed LazySeq.realize re-entrancy: clear thunk BEFORE calling it (prevents infinite recursion)
56-
- Root cause: thunk evaluation could re-enter realize on same LazySeq before result was cached
57-
- Fixes: repeat, cycle, interpose, repeatedly, iterate — all infinite lazy-seq patterns
58-
- Fixed cons to seq-ify rest argument (JVM RT.cons compatible)
59-
- Only non-seq types (vector, set, map, hash_map, string) get converted
60-
- Seq types (nil, list, cons, lazy_seq, chunked_cons) pass through
61-
- Fixes (cons 1 #{2 3}) → (1 2 3) instead of (1 . #{2 3})
62-
- Fixed builtins table count test (42→43)
63-
- Result: sequences 393/394 pass (was crashing), transducers 97/99 (was 97/99)
64-
- Both VM + TreeWalk verified
53+
41.5: Bug fixes and edge cases (round 3).
54+
- Fixed peephole optimizer not updating try_begin jump offset (compiler.zig)
55+
- Root cause: when superinstructions removed instructions, try_begin operand wasn't remapped
56+
- Caused try/catch to silently bypass catch handler in specific patterns
57+
- Fix: add .try_begin to jump fixup pass alongside .jump/.jump_if_false/.jump_back
58+
- Fixed sorted collection support:
59+
- TransientHashSet: preserve comparator through transient/persistent cycle
60+
- with-meta for sets/maps: preserve comparator field
61+
- sorted? predicate: check comparator != null (was always false)
62+
- into: bypass transient path for sorted collections, use reduce conj
63+
- into 3-arity: match JVM (persistent! in rf completion, not outside transduce)
64+
- Fixed take 0 returning nil instead of empty list (= (take 0 x) () now true)
65+
- Fixed case macro throwing plain string instead of ex-info
66+
- Result: control 157/157, sequences 593/593, transducers 100/100 (all 0 errors)
6567

6668
## Known Issues
6769

src/clj/clojure/core.clj

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,7 @@
10851085
pairs)
10861086
~@(if default
10871087
(list true default)
1088-
(list true `(throw (str "No matching clause: " ~gexpr))))))))
1088+
(list true `(throw (ex-info (str "No matching clause: " ~gexpr) {}))))))))
10891089

10901090
(defmacro condp
10911091
"Takes a binary predicate, an expression, and a set of clauses.
@@ -1648,13 +1648,21 @@
16481648
([] [])
16491649
([to] to)
16501650
([to from]
1651-
(if (or (vector? to) (map? to) (set? to))
1651+
(if (and (or (vector? to) (map? to) (set? to)) (not (sorted? to)))
16521652
(with-meta (persistent! (reduce conj! (transient to) from)) (meta to))
1653-
(reduce conj to from)))
1653+
(if (or (map? to) (set? to))
1654+
(with-meta (reduce conj to from) (meta to))
1655+
(reduce conj to from))))
16541656
([to xform from]
1655-
(if (or (vector? to) (map? to) (set? to))
1656-
(with-meta (persistent! (transduce xform conj! (transient to) from)) (meta to))
1657-
(transduce xform conj to from))))
1657+
(if (and (or (vector? to) (map? to) (set? to)) (not (sorted? to)))
1658+
(let [tm (meta to)
1659+
rf (fn
1660+
([coll] (-> (persistent! coll) (with-meta tm)))
1661+
([coll v] (conj! coll v)))]
1662+
(transduce xform rf (transient to) from))
1663+
(if (or (map? to) (set? to))
1664+
(with-meta (transduce xform conj to from) (meta to))
1665+
(transduce xform conj to from)))))
16581666

16591667
(defn- preserving-reduced
16601668
[rf]

src/common/builtin/metadata.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ pub fn withMetaFn(allocator: Allocator, args: []const Value) anyerror!Value {
166166
.map => blk: {
167167
const m = obj.asMap();
168168
const new_map = try allocator.create(PersistentArrayMap);
169-
new_map.* = .{ .entries = m.entries, .meta = meta_ptr };
169+
new_map.* = .{ .entries = m.entries, .meta = meta_ptr, .comparator = m.comparator };
170170
break :blk Value.initMap(new_map);
171171
},
172172
.hash_map => blk: {
@@ -184,7 +184,7 @@ pub fn withMetaFn(allocator: Allocator, args: []const Value) anyerror!Value {
184184
.set => blk: {
185185
const s = obj.asSet();
186186
const new_set = try allocator.create(@import("../collections.zig").PersistentHashSet);
187-
new_set.* = .{ .items = s.items, .meta = meta_ptr };
187+
new_set.* = .{ .items = s.items, .meta = meta_ptr, .comparator = s.comparator };
188188
break :blk Value.initSet(new_set);
189189
},
190190
.fn_val => blk: {

src/common/builtin/predicates.zig

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,8 +535,11 @@ fn reversiblePred(_: Allocator, args: []const Value) anyerror!Value {
535535
/// (sorted? x) — Returns true if coll implements Sorted.
536536
fn sortedPred(_: Allocator, args: []const Value) anyerror!Value {
537537
if (args.len != 1) return err.setErrorFmt(.eval, .arity_error, .{}, "Wrong number of args ({d}) passed to sorted?", .{args.len});
538-
// No sorted collections in ClojureWasm yet
539-
return Value.false_val;
538+
return Value.initBoolean(switch (args[0].tag()) {
539+
.set => args[0].asSet().comparator != null,
540+
.map => args[0].asMap().comparator != null,
541+
else => false,
542+
});
540543
}
541544

542545
/// (record? x) — Returns true if x is a record.

src/common/builtin/sequences.zig

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,13 @@ pub fn zigLazyTakeFn(allocator: Allocator, args: []const Value) anyerror!Value {
390390
.float => if (n_val.asFloat() <= 0) 0 else @intFromFloat(n_val.asFloat()),
391391
else => return err.setErrorFmt(.eval, .type_error, .{}, "__zig-lazy-take requires numeric n", .{}),
392392
};
393-
if (n == 0) return Value.nil_val;
393+
if (n == 0) {
394+
// Return empty list (not nil) to match JVM behavior.
395+
// (= (take 0 x) ()) is true in JVM Clojure.
396+
const empty = try allocator.create(PersistentList);
397+
empty.* = .{ .items = &.{} };
398+
return Value.initList(empty);
399+
}
394400
const meta = try allocator.create(LazySeq.Meta);
395401
meta.* = .{ .lazy_take = .{ .n = n, .source = args[1] } };
396402
const ls = try allocator.create(LazySeq);

src/common/bytecode/compiler.zig

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,14 @@ fn peepholeOptimize(
13601360
const new_target = ip_map[old_target];
13611361
instr.operand = @intCast(@as(i32, @intCast(nip)) + 1 - @as(i32, new_target));
13621362
},
1363+
.try_begin => {
1364+
// try_begin operand is a forward offset to catch handler (same as jump)
1365+
const old_ip2: usize = old_ip_for_new[nip];
1366+
const old_target: usize = old_ip2 + 1 + instr.operand;
1367+
const clamped = @min(old_target, n);
1368+
const new_target = ip_map[clamped];
1369+
instr.operand = @intCast(@as(i32, new_target) - @as(i32, @intCast(nip)) - 1);
1370+
},
13631371
else => {},
13641372
}
13651373
}

src/common/collections.zig

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,11 @@ pub const TransientArrayMap = struct {
274274
pub const TransientHashSet = struct {
275275
items: std.ArrayList(Value) = .empty,
276276
consumed: bool = false,
277+
comparator: ?Value = null,
277278

278279
pub fn initFrom(allocator: std.mem.Allocator, source: *const PersistentHashSet) !*TransientHashSet {
279280
const ts = try allocator.create(TransientHashSet);
280-
ts.* = .{};
281+
ts.* = .{ .comparator = source.comparator };
281282
try ts.items.appendSlice(allocator, source.items);
282283
return ts;
283284
}
@@ -317,7 +318,7 @@ pub const TransientHashSet = struct {
317318
const items = try allocator.alloc(Value, self.items.items.len);
318319
@memcpy(items, self.items.items);
319320
const s = try allocator.create(PersistentHashSet);
320-
s.* = .{ .items = items };
321+
s.* = .{ .items = items, .comparator = self.comparator };
321322
return s;
322323
}
323324
};

0 commit comments

Comments
 (0)