Skip to content

Commit 2bd48b1

Browse files
Haofeiclaude
andcommitted
rss: nested generics, sound payload-variant construction, borrowed-match extraction, callable clone
- RSS-12: nested generic type-args in method calls (parser angle-depth fix) - RSS-13: user sum payload-variant construction — resolve (hir) + lower (rust_lower qualified Enum::Variant{..}), now fully typed & checked: sum-type inference, named-field-only (RS0201), arity/unknown/duplicate/missing coverage (RS0203/4/5), per-field value-type check (RS0207), field-type-aware lowering (Int32 -> 1i32) - RSS-14: borrowed-match payload extraction — force-borrow field-place scrutinees, deref-shadow Copy payloads (let v = *v) so read-param matches bind without move/mismatch - RSS-15: callable explicit .clone() for derives(Clone) user types; gate mirrors compute_derive_attr (empty-derive non-resource cloneable, explicit-without-Clone/resource not); excluded from the read-receiver re-borrow arg rule - docs: BUGS.md RSS-12..15 (+ follow-up rounds); v0.6 spec variant-construction note; fixture comment Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2909417 commit 2bd48b1

7 files changed

Lines changed: 423 additions & 19 deletions

File tree

BUGS.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,85 @@ fn main() -> Unit {
216216
never nests generics more than a handful of levels, and the item is rated LOW / non-correctness.
217217
The risk of regressing generics resolution was judged disproportionate to the benefit, so the fix
218218
is left for a dedicated change rather than bundled with the RSS-1…RSS-10 correctness fixes.
219+
220+
---
221+
222+
## Added during tinygrad-port work (2026-06-11)
223+
224+
### RSS-12 — [FIXED] — Nested generic type-arguments in method calls parsed as comparisons
225+
- **Source:** `crates/rsscript/src/syntax/parser.rs` `find_top_level_operator` (~:3031).
226+
- `List.new<List<Int>>()`, `List.get<List<Int>>(...)`, `List.len<List<Int>>(...)` all failed with
227+
RS0015 "unsupported syntax". The binary-operator splitter tracked `angle_depth` asymmetrically:
228+
it decremented on a nested inner `>` but never incremented on the nested inner `<` (the inner
229+
`<` of `List<Int>` is not a generic-open per `is_generic_angle_open`, which requires the matching
230+
`>` to be followed by `.`/`(`), so `angle_depth` hit 0 one `>` early and the outer `>` was parsed
231+
as a `Greater` comparison → the call collapsed to `Expr::Unknown`.
232+
- **Fix:** inside a generic argument list (`angle_depth > 0`), also `angle_depth += 1` on `<`. Inside
233+
a type-argument list `<` is unambiguously a nested generic open, never a comparison.
234+
- **Verified:** `List<List<Int>>` build/index/len/param all parse, typecheck, and run.
235+
236+
### RSS-13 — [FIXED] — User sum payload-variant construction did not resolve/lower
237+
- Previously documented in fixtures as "user payload-variant construction is not yet executable":
238+
declaring and `match`ing payload variants worked, but constructing one (`ArgInt(value: 5)`,
239+
`Shape.Circle(radius: 5)`) failed with RS0206 "does not resolve" / a backend "cannot find tuple
240+
variant" error.
241+
- **Sources / fix (two sites):**
242+
1. `crates/rsscript/src/hir.rs` `resolve_call` (~:762): also resolve a call as
243+
`CallResolution::EnumVariant` when `self.sum_type_for_variant(name).is_some()` (not only the
244+
builtins Ok/Err/Some/None). The reg-VM already lowered `MakeVariant` via `sum_variant_fields`.
245+
2. `crates/rsscript/src/rust_lower/lowerer.rs` constructor lowering (~:2412): emit the qualified,
246+
struct-style form `Enum::Variant { field: value, ... }` (nullary: `Enum::Variant`) for user sum
247+
variants, matching the lowered enum (whose payload variants use named fields), instead of the
248+
bare/tuple fall-through.
249+
- **Verified:** construct + `match`/discriminate + extract (via owned/`take` scrutinee) all run.
250+
- **Known follow-up:** extracting a *Copy* payload (Int/Float) directly from a *borrowed* (`read`)
251+
match still needs the binding deref'd (use an owned `take`/cloned scrutinee for now).
252+
253+
### RSS-15 — [ADDED] — Explicit, callable `.clone()` for `derives(Clone)` types
254+
- Previously only implicit clone existed (e.g. `read` args retained into a collection); there was no
255+
surface syntax to deep-copy a user value, and `derives(Copy)` on a sum is rejected. This blocked
256+
rebuilding immutable graph nodes (e.g. a UOp simplifier).
257+
- **Added (kept implicit clone):**
258+
- `hir.rs` `resolve_receiver_call`: when no method candidate resolves and the method is `clone`,
259+
synthesize a builtin sig `(self: read T) -> fresh T` so `x.clone()` resolves and types as `T`.
260+
- `rust_lower/lowerer.rs` receiver-call lowering: for a user struct/sum receiver, emit
261+
`{receiver}.clone()` (Rust's derived Clone = deep copy). Gated to user types so builtins
262+
(JSON/Map/List) keep their own clone lowering (`json_clone`, …).
263+
- `rust_lower/lowerer.rs` `lower_call_arg_for_expected_type`: exclude `clone` from the
264+
"`read` receiver-call result is re-borrowed" rule — `.clone()` yields an owned value, so passing
265+
it to a by-value param must not add `&` (was emitting `&x.clone()` into an owned param).
266+
- **Verified:** clone of struct/sum/field; clone passed to a by-value param; whole rss suite green.
267+
268+
### RSS-13 follow-ups — [FIXED] — payload-variant construction is now typed & checked
269+
- (Audit found the initial RSS-13 resolved variant calls but did not type/check them.)
270+
- **P1 type:** `hir.rs` `infer_enum_variant_type` now returns the variant's sum type for user variants
271+
(was only Some/Ok/Err), so `Number(value: 5)` has type `Token` and misuse (e.g. passing it to a
272+
`read String` param) is caught (RS0207) instead of emitting invalid Rust.
273+
- **P1 check / no panic:** `checks/calls.rs` `check_enum_variant_form` validates user-variant
274+
construction against declared fields — unknown field name (RS0203), wrong arity (RS0203/RS0204) —
275+
so a malformed constructor (`Number(1, 2)`, `Number(bad: 5)`) is a checker error, not an
276+
out-of-bounds panic in the lowerer.
277+
- **P2 field types:** `rust_lower/lowerer.rs` variant construction lowers each arg against its declared
278+
field type (mirroring struct ctors) and resolves fields by name bounds-safely. Note: once round-2
279+
value-type checking landed, `Tiny(value: 1)` with `value: Int32` is *rejected* at check (RS0207, an
280+
`Int` literal is not `Int32`) — same as struct constructors; the field-type lowering applies to
281+
values that already have the field's type (e.g. an `Int32`-typed binding), which lower without a cast.
282+
283+
### RSS-13 follow-ups (round 2) — [FIXED] — duplicate fields & field-value types
284+
- (Second audit: the round-1 variant checker tracked names/counts but not duplicates or value types.)
285+
- **Exactly-once coverage:** `check_enum_variant_form` now tracks per-field coverage — duplicate field
286+
(RS0205), unknown field (RS0203), too many (RS0203), and any unfilled field (RS0204). So
287+
`Both(left: 1, left: 2)` reports duplicate `left` + missing `right` (was: passed check, then E0062).
288+
- **Value types:** each variant field value is type-checked against its declared field type (reusing
289+
`argument_type_matches` + the JSON/Map/List literal acceptances, like binding-payload checks). So
290+
`Number(value: "x")` for `value: Int` reports RS0207 (was: passed check, then E0308).
291+
292+
### RSS-15 / RSS-13 follow-ups (round 3) — [FIXED] — clone gated on `Clone`; variants are named-only
293+
- **`.clone()` only for `Clone`-deriving types:** the synthesized clone previously resolved for *any*
294+
user type, so `struct Boxy derives(Eq)` passed `check` then failed Rust with E0599. Now `Hir` tracks
295+
`clone_types` (declared types whose derive list includes `Clone`); `resolve_receiver_call` synthesizes
296+
`clone` only for those (non-user receivers keep prior behavior), and rust_lower emits `.clone()` only
297+
when `type_derives_clone(root)`. A declared type without `Clone` now reports RS0206 at check time.
298+
- **Variants are named-field-only:** `check_enum_variant_form` rejects any unnamed payload arg
299+
(RS0201), matching the v0.6 spec (variants use the same named-field construction form as structs).
300+
`Number(5)` is now a checker error instead of lowering to `Token::Number { value: 5i64 }`.

crates/rsscript/src/checks/calls.rs

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1721,6 +1721,109 @@ fn check_enum_variant_form(
17211721
call_span: &Span,
17221722
) {
17231723
let variant = callee_name(callee);
1724+
// User sum variants: validate construction against the variant's declared fields. Each field
1725+
// must be supplied exactly once (no unknown name, no duplicate, none missing) and each value is
1726+
// type-checked against its declared field type — so a malformed/ill-typed constructor is a
1727+
// checker error rather than a lowerer panic or a mapped rustc error.
1728+
if let Some(fields) = analyzer.hir.sum_variant_fields(&variant).map(|fields| {
1729+
fields
1730+
.iter()
1731+
.map(|f| (f.name.clone(), f.type_name.clone()))
1732+
.collect::<Vec<(String, String)>>()
1733+
}) {
1734+
// Variants use the same named-field construction form as structs (per the v0.6 spec);
1735+
// positional/unnamed payload args are not allowed.
1736+
if let Some(unnamed) = args.iter().find(|arg| arg.name.is_none()) {
1737+
analyzer.diagnostics.push(Diagnostic::error(
1738+
code::UNNAMED_ARGUMENT,
1739+
format!(
1740+
"variant `{variant}` must be constructed with named fields, e.g. `{variant}(field: value)`."
1741+
),
1742+
unnamed.span.clone(),
1743+
"variant field must be named",
1744+
));
1745+
return;
1746+
}
1747+
let mut seen = vec![false; fields.len()];
1748+
for (index, arg) in args.iter().enumerate() {
1749+
// Resolve which field this arg targets: by name if given, else positionally.
1750+
let field_idx = match &arg.name {
1751+
Some(name) => match fields.iter().position(|(fname, _)| fname == name) {
1752+
Some(i) => i,
1753+
None => {
1754+
analyzer.diagnostics.push(Diagnostic::error(
1755+
code::UNKNOWN_ARGUMENT,
1756+
format!("variant `{variant}` has no field `{name}`."),
1757+
arg.span.clone(),
1758+
"unknown variant field",
1759+
));
1760+
continue;
1761+
}
1762+
},
1763+
None if index < fields.len() => index,
1764+
None => {
1765+
analyzer.diagnostics.push(Diagnostic::error(
1766+
code::UNKNOWN_ARGUMENT,
1767+
format!(
1768+
"variant `{variant}` has {} field(s) but {} were given.",
1769+
fields.len(),
1770+
args.len()
1771+
),
1772+
arg.span.clone(),
1773+
"too many variant fields",
1774+
));
1775+
continue;
1776+
}
1777+
};
1778+
if seen[field_idx] {
1779+
analyzer.diagnostics.push(Diagnostic::error(
1780+
code::DUPLICATE_ARGUMENT,
1781+
format!(
1782+
"variant `{variant}` field `{}` is provided more than once.",
1783+
fields[field_idx].0
1784+
),
1785+
arg.span.clone(),
1786+
"duplicate variant field",
1787+
));
1788+
continue;
1789+
}
1790+
seen[field_idx] = true;
1791+
// Type-check the value against the declared field type (mirrors binding-payload checks:
1792+
// accept matching JSON/Map/List literals, skip unresolved generics, else require a match).
1793+
let expected = fields[field_idx].1.as_str();
1794+
if json_value_accepts_literal(expected, &arg.value)
1795+
|| check_map_literal_type(analyzer, expected, &arg.value, "variant field")
1796+
|| check_list_literal_type(analyzer, expected, &arg.value, "variant field")
1797+
{
1798+
continue;
1799+
}
1800+
if let Some(actual) = hir_expr_type_name(&arg.value)
1801+
&& !unresolved_generic_type(actual)
1802+
&& !argument_type_matches(expected, actual)
1803+
{
1804+
analyzer.diagnostics.push(Diagnostic::error(
1805+
code::ARGUMENT_TYPE_MISMATCH,
1806+
format!(
1807+
"variant `{variant}` field `{}` has type `{actual}`, expected `{expected}`.",
1808+
fields[field_idx].0
1809+
),
1810+
hir_expr_span(&arg.value).clone(),
1811+
"variant field type mismatch",
1812+
));
1813+
}
1814+
}
1815+
for (i, provided) in seen.iter().enumerate() {
1816+
if !provided {
1817+
analyzer.diagnostics.push(Diagnostic::error(
1818+
code::MISSING_ARGUMENT,
1819+
format!("variant `{variant}` is missing field `{}`.", fields[i].0),
1820+
call_span.clone(),
1821+
"missing variant field",
1822+
));
1823+
}
1824+
}
1825+
return;
1826+
}
17241827
let valid = match variant.as_str() {
17251828
"Ok" | "Err" | "Some" => args.len() == 1 && args[0].name.is_none(),
17261829
"None" | "Result" | "Option" => false,

crates/rsscript/src/hir.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,8 @@ pub struct HirFunctionBody {
460460
pub struct Hir {
461461
signatures: HashMap<String, FunctionSig>,
462462
types: HashMap<String, TypeInfo>,
463+
// user-declared types whose derive list includes `Clone` (gates the synthesized `.clone()`)
464+
clone_types: HashSet<String>,
463465
fields_by_name: HashMap<String, Vec<FieldInfo>>,
464466
sum_variant_types: HashMap<String, String>,
465467
sum_variant_fields: HashMap<String, Vec<FieldInfo>>,
@@ -614,6 +616,14 @@ impl Hir {
614616
&type_decl.name,
615617
&type_decl.span,
616618
);
619+
// Mirror rust_lower's derive emission: an omitted derive list defaults to
620+
// Debug+Clone for non-resource types, so those are cloneable too.
621+
if type_decl.derives.iter().any(|d| d == "Clone")
622+
|| (type_decl.derives.is_empty()
623+
&& type_decl.kind != TypeKind::Resource)
624+
{
625+
self.clone_types.insert(type_decl.name.clone());
626+
}
617627
self.insert_type(type_info_from_decl(type_decl));
618628
}
619629
Item::SumType(sum) => {
@@ -636,6 +646,10 @@ impl Hir {
636646
fields_ordered: Vec::new(),
637647
fields: HashMap::new(),
638648
};
649+
// Sums are never resources; an omitted derive list defaults to Debug+Clone.
650+
if sum.derives.is_empty() || sum.derives.iter().any(|d| d == "Clone") {
651+
self.clone_types.insert(sum.name.clone());
652+
}
639653
self.insert_type(type_info);
640654
for variant in &sum.variants {
641655
self.sum_variant_types
@@ -760,7 +774,11 @@ impl Hir {
760774

761775
pub fn resolve_call(&self, callee: &Callee) -> CallResolution {
762776
let call_name = callee_name(callee);
763-
if is_enum_variant_call(call_name) {
777+
// Builtin variants (Ok/Err/Some/None) and user-declared sum variants are both
778+
// constructor calls. The reg-VM lowerer already builds user payload variants via
779+
// `sum_variant_fields`; recognizing them here lets construction resolve instead of
780+
// being reported as an unknown callee.
781+
if is_enum_variant_call(call_name) || self.sum_type_for_variant(call_name).is_some() {
764782
return CallResolution::EnumVariant;
765783
}
766784

@@ -803,6 +821,43 @@ impl Hir {
803821
) -> (CallResolution, Option<String>) {
804822
let candidates = self.receiver_call_candidates(receiver_type, method, value_types);
805823
if candidates.is_empty() {
824+
// A declared (user) type only gets the synthesized `.clone()` if it derives `Clone`;
825+
// otherwise leave the call unresolved (RS0206) instead of emitting an `.clone()` that
826+
// Rust would reject (E0599). Non-user receivers keep their existing resolution.
827+
let clone_allowed = {
828+
let root = type_root_name(receiver_type);
829+
!self.types.contains_key(root) || self.clone_types.contains(root)
830+
};
831+
if method == "clone" && clone_allowed {
832+
// Every value supports an explicit `.clone()` returning a fresh copy of the
833+
// receiver's type. The runtime already clones implicitly (e.g. `read` args stored
834+
// into a collection); this exposes that as a callable for any `derives(Clone)` type.
835+
return (
836+
CallResolution::Resolved {
837+
signature: FunctionSig {
838+
namespace: Some(type_root_name(receiver_type).to_string()),
839+
name: "clone".to_string(),
840+
is_public: true,
841+
is_async: false,
842+
is_native: false,
843+
type_params: Box::from([]),
844+
type_param_bounds: Vec::new(),
845+
params: vec![ParamSig {
846+
name: "self".to_string(),
847+
effect: Some(ParamEffect::Read),
848+
type_name: receiver_type.to_string(),
849+
}],
850+
return_type: Some(receiver_type.to_string()),
851+
returns_fresh: true,
852+
effects: Vec::new(),
853+
retained_params: HashSet::new(),
854+
is_builtin: true,
855+
},
856+
kind: ResolvedCalleeKind::BuiltinFunction,
857+
},
858+
Some(type_root_name(receiver_type).to_string()),
859+
);
860+
}
806861
return (CallResolution::Unknown, None);
807862
}
808863
if candidates.len() > 1 {
@@ -2422,8 +2477,9 @@ fn infer_enum_variant_type(
24222477
"Some" => Some(format!("Option<{}>", payload_type(args)?)),
24232478
"Ok" => Some(format!("Result<{}, E>", payload_type(args)?)),
24242479
"Err" => Some(format!("Result<T, {}>", payload_type(args)?)),
2425-
// `None` and bare `Option`/`Result` carry no inferable payload type.
2426-
_ => None,
2480+
// A user-declared sum variant constructs a value of its sum type, so a `Number(value: 5)`
2481+
// call has type `Token` — letting the normal arg/binding type checks catch misuse.
2482+
_ => hir.sum_type_for_variant(variant).map(str::to_string),
24272483
}
24282484
}
24292485

0 commit comments

Comments
 (0)