Skip to content

Commit 3a8935d

Browse files
disconcisionclaude
andcommitted
Module semicolon ID handling: partial fix and documentation
MakeTerm changes: - Use is_mod_seq to handle any number of semicolons (not just 2 items) - Only absorb IDs when body is MultiHole (avoids duplicating single-item IDs) - Fixes infinite loop that occurred with single-item modules Known limitation documented in plan: - Only first semicolon has cursor info due to nested Skel structure - Inner semicolon IDs are buried in nested MultiHole annotations - flatten_mod flattens terms but not IDs - Documented potential solutions: modify Skel, collect IDs during flatten, or accept limitation Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent cb4ca5b commit 3a8935d

File tree

2 files changed

+85
-8
lines changed

2 files changed

+85
-8
lines changed

plans/modules-phase-1.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,3 +1665,73 @@ No ID appears in multiple places, so no overwrites occur.
16651665
```
16661666

16671667
This fixes the stack overflow that occurred when ModExp's ID overlapped with the inner expression's IDs.
1668+
1669+
---
1670+
1671+
## Known Issue: Nested Semicolon IDs Not Collected
1672+
1673+
### The Problem
1674+
1675+
For modules with 3+ items like `{ let x = 1; let y = 2; let z = 3 }`, only the **first** semicolon has cursor inspector info. Subsequent semicolons show "whitespace or comment".
1676+
1677+
### Root Cause
1678+
1679+
The Skel (skeleton) system produces **nested** binary structures for same-precedence operators:
1680+
1681+
```
1682+
Bin(Bin(a_skel, [;1], b_skel), [;2], c_skel)
1683+
```
1684+
1685+
When `unsorted` in MakeTerm processes this:
1686+
1. Outer level sees `Bin(l_skel, [;2], c_skel)`
1687+
2. Recursively processes `l_skel` → produces `Mod(inner_result)` with `annotation.ids = [;1]`
1688+
3. Outer `tiles` only contains `[;2]`
1689+
4. `is_mod_seq(tiles)` returns `Some([])` - no between_kids at this level
1690+
5. Result: `all_items = [Mod(inner_result), Mod(c)]`
1691+
6. `ids(unsorted)` at outer level = `[;2]` only
1692+
1693+
The inner `;1` ID is buried inside `inner_result.annotation.ids` and never collected.
1694+
1695+
### Current State
1696+
1697+
- `flatten_mod` correctly flattens the **terms** (extracts all Mod items from nested MultiHole)
1698+
- But it does NOT collect the **IDs** from nested structures
1699+
- Only the outermost semicolon ID ends up in the Module expression's annotation
1700+
- Statics adds cursor info only for those IDs
1701+
1702+
### Potential Solutions
1703+
1704+
**Option A: Modify Skel to produce flat structures for semicolons**
1705+
1706+
Make semicolons behave like commas in tuples (if tuples are indeed flat). This would require changes to `Skel.re` to handle ModSeq specially. However, it's unclear if tuples actually produce flat structures or just happen to work for other reasons.
1707+
1708+
**Option B: Collect IDs during flattening**
1709+
1710+
Modify `flatten_mod` (or create a new function) to collect IDs from nested MultiHole annotations while flattening:
1711+
1712+
```reason
1713+
let rec flatten_mod_with_ids = (m: Mod.t): (list(Mod.t), list(Id.t)) =>
1714+
switch (m.term) {
1715+
| MultiHole(kids) =>
1716+
let results = kids |> List.filter_map(...) |> List.map(flatten_mod_with_ids);
1717+
let items = results |> List.map(fst) |> List.flatten;
1718+
let ids = m.annotation.ids @ (results |> List.map(snd) |> List.flatten);
1719+
(items, ids)
1720+
| _ => ([m], [])
1721+
};
1722+
```
1723+
1724+
Then use these collected IDs in the Module case of exp_term.
1725+
1726+
**Option C: Accept the limitation**
1727+
1728+
Document that cursor inspector works for curly braces and the first semicolon, but not subsequent semicolons. This is a minor UX issue - the type information is still correct, just not accessible via clicking all semicolons.
1729+
1730+
### Decision
1731+
1732+
Deferred. The current implementation works correctly for evaluation and type-checking. Cursor inspector works for:
1733+
- Curly braces (`{` and `}`)
1734+
- First semicolon
1735+
- All ModLet/ModType items
1736+
1737+
This is acceptable for Phase 1. A complete fix can be addressed in a future iteration.

src/haz3lcore/lang/MakeTerm.re

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,17 @@ and exp_term: unsorted => (Exp.term, list(Id.t)) = {
331331
/* ModBody absorption: inner Mod's semicolon IDs become part of Module.
332332
ID order: [curly_brace_id] @ semicolon_ids (outer first, then adopted).
333333
This ensures cursor inspector works for both curly braces AND semicolons.
334-
Special case: {} with just an EmptyHole child means empty module. */
334+
IMPORTANT: Only absorb when body is MultiHole (from semicolons).
335+
For single items, body.annotation.ids would be the ModLet/ModType ID,
336+
which is also used by the expanded Let/TyAlias - absorbing would duplicate. */
335337
switch (body) {
336338
| {term: EmptyHole, _} => ret(Module([]))
337-
| {annotation: {ids, _}, term: _} =>
339+
| {annotation: {ids, _}, term: MultiHole(_)} =>
340+
/* Multiple items: absorb semicolon IDs */
338341
adopted_ids := ids @ adopted_ids^;
342+
(Module(flatten_mod(body)), ids)
343+
| _ =>
344+
/* Single item: don't absorb (would duplicate ModLet/ModType ID) */
339345
ret(Module(flatten_mod(body)))
340346
}
341347
| (["{", "}"], [Exp(body)])
@@ -914,13 +920,14 @@ and mod_term: unsorted => TermBase.Mod.term = {
914920
}
915921
| _ => ret(hole(tm))
916922
}
917-
/* ModSeq: semicolon-separated module items */
923+
/* ModSeq: semicolon-separated module items (like tuples with commas) */
918924
| Bin(Mod(m1), tiles, Mod(m2)) =>
919-
switch (tiles) {
920-
| ([(_id, ([";"], []))], []) =>
921-
/* For now, sequence produces a multihole - will be refined in Phase 1.2 */
922-
ret(MultiHole([Mod(m1), Mod(m2)]))
923-
| _ => ret(hole(Bin(Mod(m1), tiles, Mod(m2))))
925+
switch (is_mod_seq(tiles)) {
926+
| Some(between_kids) =>
927+
/* Flatten all mod items into MultiHole, like tuples flatten into Tuple */
928+
let all_items = [Grammar.Mod(m1)] @ List.map(m => Grammar.Mod(m), between_kids) @ [Grammar.Mod(m2)];
929+
ret(MultiHole(all_items))
930+
| None => ret(hole(Bin(Mod(m1), tiles, Mod(m2))))
924931
}
925932
/* ModLet: let p = e - the pattern is inside the tile, expression is the body */
926933
| Pre(([(_id, (["let", "="], [Pat(p)]))], []), Exp(e)) =>

0 commit comments

Comments
 (0)