Skip to content

Commit 5ae327b

Browse files
Haofeiclaude
andcommitted
fix(lowering): carry let-annotation for Deque/SortedMap/SortedSet (E0282)
`lower_let_annotation`'s generic-container allowlist included List/Map/Set/ Option/Result/Channel but omitted `Deque`, `SortedMap`, and `SortedSet`. A `let v: Deque<Float> = Deque.new<Float>()` whose only use was `len()` therefore dropped its annotation and lowered to an un-inferable `VecDeque<_>` — the checker ACCEPTED it and the VM ran it, but the compiled backend failed to build (E0282: type annotations needed). That violates the "valid RSScript -> valid Rust" contract. Add the three missing containers so every generic container carries its element type into Rust. Found by the rss-testgen generative differential (full N-way incl. compiled). Regression test in checker_lowering covers all three containers. The generator's bug-workarounds (always-populate collections; collections excluded from arbitrary type positions) are dropped now that empty annotated collections lower. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b80041e commit 5ae327b

4 files changed

Lines changed: 185 additions & 9 deletions

File tree

TODO.md

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,141 @@ _Every item says **why** — if you can't fill in a why, it doesn't belong here.
88
- **Stream / lazy Iter** = the deferred performance path. Blueprint: a lazy iterator is `struct Iter[X] { next: () -> Option<X> }`, combinators wrap the next-closure → lazy fusion, zero intermediates. _Why parked / why noted:_ it's the "Stream for performance" we split off from the eager Pipeline; record the design so it's ready, **with the review-first caveats**: it's mutate-on-read / consume-once (hidden-state hazard), and prefer explicit loops over callback-iteration (even mature lazy-iterator libraries are moving away from it).
99
- Keep the `try_` surface **small** (don't generate try_ variants; let pipeline's `FalliblePipeline` cover the common case). _Why:_ error-polymorphism (inferring fallibility from the callback) would collapse the `try_` surface, but RSS pays explicit fallibility with duplication, so the price must stay bounded (Article II).
1010
- `read` omittable-default decision (§2A.3, recorded); pattern extras (`|`, range, `as`/`@`); **map-pattern via a `get(Self,K)->Option<V>` protocol** (defers protocol-dispatched patterns — much heavier semantics than struct/sum, and the interpreter doesn't need it); shared Iter protocol to collapse ×collections. _Why parked:_ not on the dogfooding critical path; revisit when a real program demands them.
11+
12+
## tinygrad port feedback
13+
14+
These items came from the tinygrad RSScript/modern-c port. They are not abstract
15+
language wish-list items: each one removes a current source-port workaround or
16+
manual wrapper.
17+
18+
### Must unblock awkward valid ports
19+
20+
- [ ] **Generated namespace isolation.** Preserve source/module/type/member
21+
identity through checking and lowering, then emit globally unique Rust symbols.
22+
_Why:_ valid tinygrad names such as `helpers.count`, `device.count`,
23+
`helpers.T`, and `tensor.T` currently collide because RSS lowers too many
24+
things into one Rust item namespace. The port has to invent names like
25+
`helpers_count` and map them back in tooling.
26+
_Acceptance:_ two modules can define the same final name; a module-level value,
27+
type alias, struct, method, and generated helper with related names cannot
28+
collide after Rust lowering; diagnostics report the RSS source symbol, not only
29+
the lowered Rust name.
30+
31+
- [ ] **Stable source-qualified symbol identity.** Store and expose a symbol's
32+
module path, source qualified name, kind, visibility, and lowered backend name.
33+
_Why:_ port tooling should compare `helpers.py::count` to
34+
`helpers.rss::count`, not guess from bare names.
35+
_Acceptance:_ RSS can emit a checked symbol inventory suitable for portman:
36+
`module`, `qualname`, `kind`, `source_span`, `lowered_name`.
37+
38+
- [ ] **Class/static member support.** Model class attributes, associated
39+
constants, and static methods directly.
40+
_Why:_ tinygrad uses symbols such as `Tensor.train`, `Device.DEFAULT`,
41+
`dtypes.float`, `UOp.const`, and `UPat.var` as class/static members. Porting
42+
them as free functions or dummy globals loses structure and creates name
43+
conflicts.
44+
_Acceptance:_ RSS supports declaring and lowering type-associated values and
45+
methods without requiring separate global wrappers.
46+
47+
- [ ] **Type aliases and generic aliases without value placeholders.** Add
48+
source-visible type alias declarations that do not create value namespace
49+
entries unless explicitly requested.
50+
_Why:_ Python `TypeVar` and alias-like symbols currently become dummy constants
51+
in the port just so coverage tooling can see them.
52+
_Acceptance:_ aliases can be generic, can reference imported types, and appear
53+
in the symbol inventory as type-level symbols.
54+
55+
- [ ] **Method/property lowering ergonomics.** Provide a simple way to model
56+
Python-style getter properties and method-like computed fields.
57+
_Why:_ tinygrad has many small property methods where the implementation is
58+
simple but the RSS surface needs repetitive wrappers.
59+
_Acceptance:_ getter-style members lower predictably, can borrow `self`, and
60+
preserve member identity for diagnostics and inventory.
61+
62+
### Reduce manual translation volume
63+
64+
- [ ] **Container operation coverage.** Make common `List`, `Map`, `Set`,
65+
`Bytes`, `Buffer`, tuple, and optional operations straightforward and
66+
consistently checked.
67+
_Why:_ tinygrad code is dense with `len`, `append`, `pop`, indexing, slicing,
68+
membership, iteration, and clone/copy patterns. Every missing builtin creates a
69+
hand-written helper.
70+
_Acceptance:_ supported operations either compile and lower to idiomatic Rust
71+
or fail with an error that names the unsupported operation and type.
72+
73+
- [ ] **Clone/copy consistency for builtin value types.** Continue the
74+
`List`/`Bytes`/`Buffer` `.clone()` direction and make the checker/lowerer agree
75+
exactly on every builtin clone target.
76+
_Why:_ the tinygrad port previously hit Rust `E0425` because the checker
77+
accepted a builtin clone that lowering emitted as a missing helper.
78+
_Acceptance:_ checker-approved builtin clones always lower to valid Rust, and
79+
checker-rejected clones state the supported alternatives.
80+
81+
- [ ] **Option/null ergonomics.** Add concise checked forms for option tests,
82+
unwrap-with-default, early return on none, and optional field access.
83+
_Why:_ tinygrad has many maybe-present values; verbose option plumbing makes
84+
direct ports drift away from the source.
85+
_Acceptance:_ common option patterns require no custom helpers and preserve
86+
ownership/borrow rules clearly.
87+
88+
- [ ] **Pattern matching over variants, options, constants, tuples, and lists.**
89+
_Why:_ the UOp/UPat rewrite system is pattern-heavy. Lowering it as nested
90+
conditionals would be slow to write and hard to audit.
91+
_Acceptance:_ match arms can bind values, match enum/variant tags, constants,
92+
tuples, and optional values; exhaustiveness or explicit fallback is checked.
93+
94+
- [ ] **Callable and limited closure support.** Support function values and
95+
simple captures well enough for callbacks and rewrite rules.
96+
_Why:_ tinygrad uses lambdas/callback-like matchers. Without callable support,
97+
the port must manually defunctionalize logic into many structs.
98+
_Acceptance:_ RSS can pass named functions and simple closures to higher-order
99+
helpers with checked capture ownership.
100+
101+
- [ ] **Default arguments and construction helpers.** Provide a first-class way
102+
to express common defaulted constructor/helper parameters.
103+
_Why:_ tinygrad APIs use many Python defaults. Manual overload/wrapper sets
104+
inflate the port.
105+
_Acceptance:_ defaulted parameters lower deterministically and appear in docs
106+
or symbol inventory without changing the call ABI unexpectedly.
107+
108+
- [ ] **Tuple and destructuring ergonomics.** Make multiple-return and unpacking
109+
patterns concise.
110+
_Why:_ tinygrad frequently returns and unpacks grouped values.
111+
_Acceptance:_ tuple literals, tuple returns, and local destructuring work with
112+
ownership and borrowing tracked normally.
113+
114+
- [ ] **String and bytes utility coverage.** Fill gaps for split/join/search,
115+
prefix/suffix checks, formatting, byte conversion, and cheap slicing where
116+
supported.
117+
_Why:_ device/runtime/autogen and file-path code are string/bytes-heavy.
118+
_Acceptance:_ common string/bytes operations compile without local helper
119+
shims, or fail with precise unsupported-operation diagnostics.
120+
121+
### Tooling and diagnostics
122+
123+
- [ ] **RSS-to-Rust diagnostic provenance.** Carry source spans and symbol
124+
identity into generated Rust and back through compiler errors.
125+
_Why:_ when Rust fails, the port currently has to infer which RSS declaration
126+
produced the bad item.
127+
_Acceptance:_ a Rust backend error reports the RSS file, source span, source
128+
symbol, and lowered Rust symbol.
129+
130+
- [ ] **Explicit lowered-name escape hatch.** Add an attribute for rare cases
131+
where a declaration needs a pinned backend name.
132+
_Why:_ useful during compiler transition and for FFI/autogen boundaries.
133+
_Acceptance:_ something like `#[lower_name = "helpers__count"]` is checked for
134+
uniqueness and reflected in the symbol inventory.
135+
136+
- [ ] **Generated/internal helper namespace.** Keep compiler-generated helpers
137+
separate from user declarations.
138+
_Why:_ helper names should never consume source-level names or force a port to
139+
avoid otherwise valid identifiers.
140+
_Acceptance:_ generated helper names are always compiler-reserved/mangled and
141+
never collide with user-visible module/type/member names.
142+
143+
- [ ] **External/FFI declaration ergonomics.** Make copied runtime/autogen and
144+
device boundaries easy to declare without large wrapper files.
145+
_Why:_ tinygrad runtime/autogen should mostly be copied or bound, not manually
146+
reimplemented.
147+
_Acceptance:_ external functions/types can be declared compactly, checked, and
148+
included in the symbol inventory with provenance.

crates/rss-testgen/src/generator.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,7 @@ impl<'a> Generator<'a> {
6868
/// available. Compound payloads are restricted to scalars to keep the surface
6969
/// finite and generation simple.
7070
fn pick_type(&mut self) -> Ty {
71-
// Collections are deliberately *not* pickable here: an empty `C.new<T>()`
72-
// in an inferred position lowers to a Rust `C::new()` whose element type
73-
// can't be inferred (E0282) when only `len()` is observed. Collections are
74-
// instead emitted, always populated, by `gen_collection_stmt`.
75-
match self.seed.weighted(&[10, 2, 2, 3, 3]) {
71+
match self.seed.weighted(&[10, 2, 2, 3, 3, 2, 2, 2, 1]) {
7672
0 => self.pick_scalar(),
7773
1 => Ty::Option(Box::new(self.pick_scalar())),
7874
2 => Ty::Result(Box::new(self.pick_scalar()), Box::new(Ty::String)),
@@ -84,6 +80,13 @@ impl<'a> Generator<'a> {
8480
4 if !self.sums.is_empty() => {
8581
Ty::Sum(self.sums[self.seed.choice(self.sums.len())].name.clone())
8682
}
83+
5 => Ty::List(Box::new(self.pick_scalar())),
84+
6 => Ty::Map(
85+
Box::new(self.pick_hashable_scalar()),
86+
Box::new(self.pick_scalar()),
87+
),
88+
7 => Ty::Set(Box::new(self.pick_hashable_scalar())),
89+
8 => Ty::Deque(Box::new(self.pick_scalar())),
8790
_ => self.pick_scalar(),
8891
}
8992
}
@@ -376,10 +379,10 @@ impl<'a> Generator<'a> {
376379
/// continuation lines are pre-indented to match the caller's block.
377380
fn gen_collection_stmt(&mut self, scope: &mut Scope) -> String {
378381
let name = self.fresh_var();
379-
// Always at least one element: a non-empty collection lets the Rust
380-
// backend infer the element type from the pushed literal (an empty
381-
// `C.new<T>()` observed only via `len()` is un-inferable — E0282).
382-
let count = 1 + self.seed.choice(3); // 1..=3 elements
382+
// Empty collections are fine now that typed `let` bindings carry their
383+
// annotation through lowering (the Deque/SortedMap/SortedSet gap that made
384+
// an empty `C.new<T>()` un-inferable — E0282 — is fixed in the lowerer).
385+
let count = self.seed.choice(4); // 0..=3 elements
383386
let (ty, new_expr, mut inserts, len_expr): (Ty, String, Vec<String>, String) =
384387
match self.seed.choice(4) {
385388
0 => {

crates/rsscript/src/rust_lower/lowerer.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4436,6 +4436,9 @@ impl<'a> RustLowerer<'a> {
44364436
"List",
44374437
"Map",
44384438
"Set",
4439+
"Deque",
4440+
"SortedMap",
4441+
"SortedSet",
44394442
"Option",
44404443
"Result",
44414444
"ResourcePool",

crates/rsscript/tests/checker_lowering/types.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,38 @@ fn run() -> Result<Unit, ChannelError> {
586586
);
587587
}
588588

589+
#[test]
590+
fn typed_let_emits_deque_and_sorted_container_annotations() {
591+
// Regression (found by rss-testgen): `Deque`, `SortedMap`, and `SortedSet`
592+
// were missing from the generic-container annotation allowlist, so a
593+
// `let v: Deque<Float> = Deque.new<Float>()` whose only use was `len()`
594+
// dropped its annotation and lowered to an un-inferable `VecDeque<_>`
595+
// (E0282) — accepted by the checker but un-compilable. Every generic
596+
// container must carry its annotation into Rust.
597+
for (decl, expected) in [
598+
(
599+
"let v: Deque<Float> = Deque.new<Float>()",
600+
"let v: std::collections::VecDeque<f64> =",
601+
),
602+
(
603+
"let v: SortedMap<Int, String> = SortedMap.new<Int, String>()",
604+
"let v: std::collections::BTreeMap<i64, String> =",
605+
),
606+
(
607+
"let v: SortedSet<Int> = SortedSet.new<Int>()",
608+
"let v: std::collections::BTreeSet<i64> =",
609+
),
610+
] {
611+
let source = format!("fn main() -> Unit {{\n {decl}\n return Unit\n}}\n");
612+
let lowered = lower_source_to_rust("typed-container.rss", &source)
613+
.unwrap_or_else(|diagnostics| panic!("{decl} should lower: {diagnostics:?}"));
614+
assert!(
615+
lowered.contains(expected),
616+
"`{decl}` should emit `{expected}`, got:\n{lowered}"
617+
);
618+
}
619+
}
620+
589621
#[test]
590622
fn rust_lowering_classifies_dependency_interface_types() {
591623
// Review #8: a class declared in a *dependency* interface (not the current

0 commit comments

Comments
 (0)