Skip to content

Commit 302589f

Browse files
Haofeiclaude
andcommitted
fix(checker): reject provably-uninferable unused Ok/Err/None bindings (RS0034)
A bare `let v = Ok(x)` / `Err(x)` / `None` leaves a type parameter open (the Result error type, the Result ok type, or the Option value type). The checker accepted it, the VM ran it, but it failed to lower to valid Rust (E0282: type annotations needed) — surfacing as a backend RS1101 instead of a frontend RSScript error, violating the "RSScript owns semantics" contract. The analyzer is string-based (it leaks literal `E`/`T` placeholders and never resolves them from usage), so it cannot distinguish the un-lowerable case from a valid constrained-by-use one — EXCEPT by one sound signal: whether the binding is ever used. Unused => the placeholder can never be pinned => guaranteed E0282 => sound to reject. Used => a later use may pin it (e.g. core-properties' `let input = Ok(value); match Result.map(input) { Err(e) => <e as String> }`), so it is left alone. The "not annotated" test is `type_name == value_type_name` (the HIR sets them equal exactly when the user wrote no annotation). New RS0034 (+ explanation). Found by the rss-testgen generative differential. Regression: a fail fixture plus a soundness test asserting it fires on unused Ok/Err/None and never on Some / annotated / constrained-by-use bindings. The full suite (1011) stays green — no existing program is newly rejected. A complete fix (reject only when *no* downstream use pins the param, for used bindings too) needs real free-type-variable inference, which the string-based analyzer lacks; that is a tracked follow-up. The remaining used-but-unpinned cases still fail at the backend with a clean source-mapped RS1101. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5ae327b commit 302589f

6 files changed

Lines changed: 372 additions & 138 deletions

File tree

TODO.md

Lines changed: 0 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -8,141 +8,3 @@ _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/rsscript/src/checks/body.rs

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ pub(crate) fn check(analyzer: &mut Analyzer<'_>) {
5353
}
5454
check_resource_pool_bindings(analyzer, body);
5555
check_local_class_bindings(analyzer, body);
56+
check_uninferable_unused_bindings(analyzer, body);
5657
if let Some(block) = body.block.as_ref() {
5758
check_resource_pool_discards(analyzer, block, &mut Vec::new());
5859
let bindings: std::collections::HashMap<String, HirBindingKind> = body
@@ -1083,6 +1084,189 @@ fn collect_stmt_uses(statement: &HirStmt, uses: &mut HashSet<String>) {
10831084
}
10841085
}
10851086

1087+
/// Report `let` bindings whose type cannot be inferred (RS0034).
1088+
///
1089+
/// A bare `Ok(..)`/`Err(..)`/`None` initializer leaves a type parameter open
1090+
/// (`Result`'s error type, `Result`'s ok type, or `Option`'s value type).
1091+
/// RSScript resolves that parameter from how the binding is later *used*; if the
1092+
/// binding is never used, nothing constrains it, the type is genuinely ambiguous,
1093+
/// and the program would not lower to valid Rust (rustc `E0282`). The "never
1094+
/// used" gate is what keeps this sound — a binding used downstream may have its
1095+
/// parameter pinned there (e.g. `let r = Ok(x); ... match Result.map(r) { Err(e)
1096+
/// => use e as String }`), so only the provably-unconstrained (unused) case is
1097+
/// rejected, which never false-positives on constrained-by-use code.
1098+
fn check_uninferable_unused_bindings(
1099+
analyzer: &mut Analyzer<'_>,
1100+
body: &crate::hir::HirFunctionBody,
1101+
) {
1102+
let Some(block) = body.block.as_ref() else {
1103+
return;
1104+
};
1105+
// Every name referenced anywhere in the function, with binding structure
1106+
// ignored (unlike `collect_block_uses`, which strips locally-bound names to
1107+
// compute free variables). We need raw references so a binding used only as a
1108+
// local — `return v`, `f(read v)` — counts as used.
1109+
let mut uses = HashSet::new();
1110+
collect_all_referenced_names_block(block, &mut uses);
1111+
check_uninferable_bindings_in_block(analyzer, block, &uses);
1112+
}
1113+
1114+
fn collect_all_referenced_names_block(block: &HirBlock, uses: &mut HashSet<String>) {
1115+
for statement in &block.statements {
1116+
collect_all_referenced_names_stmt(statement, uses);
1117+
}
1118+
}
1119+
1120+
fn collect_all_referenced_names_stmt(statement: &HirStmt, uses: &mut HashSet<String>) {
1121+
match statement {
1122+
HirStmt::Let { value, .. } | HirStmt::Return { value, .. } => {
1123+
if let Some(value) = value {
1124+
collect_expr_uses(value, uses);
1125+
}
1126+
}
1127+
HirStmt::With { resource, body, .. } => {
1128+
collect_expr_uses(resource, uses);
1129+
collect_all_referenced_names_block(body, uses);
1130+
}
1131+
HirStmt::If {
1132+
condition,
1133+
then_body,
1134+
else_body,
1135+
..
1136+
} => {
1137+
collect_expr_uses(condition, uses);
1138+
collect_all_referenced_names_block(then_body, uses);
1139+
if let Some(else_body) = else_body {
1140+
collect_all_referenced_names_block(else_body, uses);
1141+
}
1142+
}
1143+
HirStmt::Loop {
1144+
condition, body, ..
1145+
} => {
1146+
if let Some(condition) = condition {
1147+
collect_expr_uses(condition, uses);
1148+
}
1149+
collect_all_referenced_names_block(body, uses);
1150+
}
1151+
HirStmt::For { iterable, body, .. } => {
1152+
collect_expr_uses(iterable, uses);
1153+
collect_all_referenced_names_block(body, uses);
1154+
}
1155+
HirStmt::Match { value, arms, .. } => {
1156+
collect_expr_uses(value, uses);
1157+
for arm in arms {
1158+
if let Some(guard) = &arm.guard {
1159+
collect_expr_uses(guard, uses);
1160+
}
1161+
collect_all_referenced_names_block(&arm.body, uses);
1162+
}
1163+
}
1164+
HirStmt::Select { arms, .. } => {
1165+
for arm in arms {
1166+
collect_expr_uses(&arm.operation, uses);
1167+
collect_all_referenced_names_block(&arm.body, uses);
1168+
}
1169+
}
1170+
HirStmt::Expr(expr) => collect_expr_uses(expr, uses),
1171+
HirStmt::Assign { target, value, .. } => {
1172+
for read in crate::hir::assign_target_reads(target) {
1173+
collect_expr_uses(read, uses);
1174+
}
1175+
collect_expr_uses(value, uses);
1176+
}
1177+
HirStmt::Break(_) | HirStmt::Continue(_) | HirStmt::Unknown(_) => {}
1178+
}
1179+
}
1180+
1181+
fn check_uninferable_bindings_in_block(
1182+
analyzer: &mut Analyzer<'_>,
1183+
block: &HirBlock,
1184+
uses: &HashSet<String>,
1185+
) {
1186+
for statement in &block.statements {
1187+
check_uninferable_bindings_in_stmt(analyzer, statement, uses);
1188+
}
1189+
}
1190+
1191+
fn check_uninferable_bindings_in_stmt(
1192+
analyzer: &mut Analyzer<'_>,
1193+
statement: &HirStmt,
1194+
uses: &HashSet<String>,
1195+
) {
1196+
match statement {
1197+
// A bare `Ok`/`Err`/`None` with no user annotation (the HIR sets
1198+
// `type_name == value_type_name` exactly when the user did not annotate —
1199+
// an annotation would override `type_name` with a placeholder-free type),
1200+
// bound to a name that is never used, so nothing pins the open parameter.
1201+
HirStmt::Let {
1202+
name,
1203+
value: Some(value),
1204+
type_name,
1205+
value_type_name,
1206+
span,
1207+
..
1208+
} if type_name == value_type_name
1209+
&& !uses.contains(name)
1210+
&& open_variant_constructor(value) =>
1211+
{
1212+
analyzer.diagnostics.push(
1213+
Diagnostic::error(
1214+
code::UNINFERABLE_BINDING_TYPE,
1215+
format!("the type of `{name}` cannot be inferred."),
1216+
span.clone(),
1217+
"uninferable binding type",
1218+
)
1219+
.with_cause(
1220+
"A bare `Ok(...)`, `Err(...)`, or `None` leaves a type parameter open, and this binding is never used, so nothing can constrain it — the type is ambiguous and would not lower to valid Rust.",
1221+
)
1222+
.with_fix(
1223+
"annotate_binding_type",
1224+
"Add a type annotation (e.g. `let v: Result<Int, String> = ...`) or remove the unused binding.",
1225+
"manual",
1226+
),
1227+
);
1228+
}
1229+
HirStmt::With { body, .. } | HirStmt::Loop { body, .. } | HirStmt::For { body, .. } => {
1230+
check_uninferable_bindings_in_block(analyzer, body, uses);
1231+
}
1232+
HirStmt::If {
1233+
then_body,
1234+
else_body,
1235+
..
1236+
} => {
1237+
check_uninferable_bindings_in_block(analyzer, then_body, uses);
1238+
if let Some(else_body) = else_body {
1239+
check_uninferable_bindings_in_block(analyzer, else_body, uses);
1240+
}
1241+
}
1242+
HirStmt::Match { arms, .. } => {
1243+
for arm in arms {
1244+
check_uninferable_bindings_in_block(analyzer, &arm.body, uses);
1245+
}
1246+
}
1247+
HirStmt::Select { arms, .. } => {
1248+
for arm in arms {
1249+
check_uninferable_bindings_in_block(analyzer, &arm.body, uses);
1250+
}
1251+
}
1252+
_ => {}
1253+
}
1254+
}
1255+
1256+
/// Whether `value` is a bare `Ok(..)`/`Err(..)`/`None` constructor — the
1257+
/// constructors that leave a type parameter unconstrained (`Some(x)` is fully
1258+
/// determined by `x`, so it is excluded).
1259+
fn open_variant_constructor(value: &HirExpr) -> bool {
1260+
match value {
1261+
HirExpr::Call {
1262+
callee: Callee::Name(name),
1263+
..
1264+
} => matches!(name.as_str(), "Ok" | "Err" | "None"),
1265+
HirExpr::Ident { name, .. } => name == "None",
1266+
_ => false,
1267+
}
1268+
}
1269+
10861270
fn collect_block_uses(block: &HirBlock, uses: &mut HashSet<String>) {
10871271
let mut block_uses = HashSet::new();
10881272
for statement in block.statements.iter().rev() {

crates/rsscript/src/diagnostic.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub mod code {
3737
pub const AWAIT_LIVE_LOCAL: &str = "RS0031";
3838
pub const PROTOCOL_NOT_SATISFIED: &str = "RS0032";
3939
pub const INTEGER_LITERAL_OUT_OF_RANGE: &str = "RS0033";
40+
pub const UNINFERABLE_BINDING_TYPE: &str = "RS0034";
4041
pub const FEATURE_VIOLATION: &str = "RS0101";
4142
pub const UNNAMED_ARGUMENT: &str = "RS0201";
4243
pub const MISSING_DATA_EFFECT: &str = "RS0202";
@@ -354,6 +355,11 @@ static DIAGNOSTIC_EXPLANATIONS: &[DiagnosticExplanation] = &[
354355
title: "missing parameter type",
355356
explanation: "Parameters must have explicit types so call effects, freshness, and resource rules can be checked against a stable signature.",
356357
},
358+
DiagnosticExplanation {
359+
code: code::UNINFERABLE_BINDING_TYPE,
360+
title: "binding type cannot be inferred",
361+
explanation: "A `let` binding whose value is a bare `Ok(...)`, `Err(...)`, or `None` leaves a type parameter open (the `Result` error type, the `Result` ok type, or the `Option` value type). RSScript normally resolves that parameter from how the binding is later used; when the binding is never used, nothing can constrain it, so the type is genuinely ambiguous and the program would not lower to valid Rust. The checker reports this in RSScript — instead of letting it surface as a backend `type annotations needed` error — and the fix is to add a type annotation (e.g. `let v: Result<Int, String> = Ok(value)`) or to remove the unused binding.",
362+
},
357363
DiagnosticExplanation {
358364
code: code::UNKNOWN_EFFECT,
359365
title: "unknown effect",

0 commit comments

Comments
 (0)