Skip to content

Commit 3896a5b

Browse files
Haofeiclaude
andcommitted
Fix review #5 (async spec drift) and #8 (dependency types in lowering)
#5: §3.1/§3.2 listed Stream/await-for as not-executable and described async as 'restricted direct await', but task_group/select/await-for/channels/streams are executable now. Updated the executable-MVP and review-only scope + async prose to match the implementation. (Assignment drift was already fixed; the reviewer cited a non-existent specs/ path.) #8: a class/resource/struct defined in a *dependency* interface was absent from the lowerer's type_kinds, so it mis-lowered (e.g. a named-field class constructed as a positional Widget(1i64) call). Now non-builtin dependency interface types are ingested into type_kinds; bundled stdlib interface types stay excluded so runtime-backed types (ProcessRequest -> rsscript_runtime::ProcessRequest) keep their qualification. Verified: dependency Widget now lowers as managed named-field construction; stdlib lowering unchanged. Regression test + rust_lowering_classifies_dependency_interface_types. Removes RSS-14. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8f2550c commit 3896a5b

4 files changed

Lines changed: 84 additions & 41 deletions

File tree

BUGS.md

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,3 @@ $BIN eval file.rss
5050
unified) and be guarded by a differential/fuzz test comparing old vs new output across many type
5151
strings — otherwise it risks silently changing which programs typecheck. That verification, not the
5252
tree itself, is the bulk of the work and the reason this stays a dedicated, separately-reviewed change.
53-
54-
### RSS-14 — [VALID · DEFERRED] — dependency-defined types not in the lowering type environment
55-
- **Source:** `crates/rsscript/src/rust_lower/lowerer.rs``RustLowerer`'s `type_kinds` map (and the
56-
`self.program.items` lookups for sum variants / fields) is built from the **current program only**;
57-
the `interface_programs` (builtin + dependency `.rssi`) are not folded in.
58-
- **Effect:** a `class`/`resource`/`struct`/`sum` declared in a *dependency* package's interface and
59-
then constructed/held/matched in the current source can typecheck against the contract but lower
60-
incorrectly, because `is_class_type`/`is_resource_type`/`field_type`/`sum_variant_fields_for_type`
61-
don't know its kind/fields. Not reproducible from a single file (`rss run` takes no `--interface`);
62-
it needs a real multi-package build.
63-
- **Why not a blanket fix:** simply ingesting every `interface_programs` `Item::Type` into `type_kinds`
64-
is **wrong** — the bundled stdlib interfaces declare *runtime-backed* types (e.g. `ProcessRequest`,
65-
which must lower as `rsscript_runtime::ProcessRequest`) as plain structs. Classifying those as local
66-
user types drops the `rsscript_runtime::` qualification and changes their lowering (verified: it
67-
breaks `rust_lowering_maps_process_request/stream/rules_config_reload` while parity stays green, i.e.
68-
a silent shape regression).
69-
- **Fix:** build a lowering type environment that ingests dependency interface types **while
70-
distinguishing runtime-backed stdlib types from genuine dependency types** (e.g. by interface
71-
origin / a runtime-binding marker), and validate with a multi-package fixture that constructs and
72-
matches a dependency-defined class/resource/sum. Deferred so this isn't rushed into a stdlib-lowering
73-
regression.

crates/rsscript/src/rust_lower/lowerer.rs

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,26 @@ impl<'a> RustLowerer<'a> {
5252
interface_programs: &[Program],
5353
) -> Self {
5454
// Type kinds (struct/class/resource) for `is_class_type`/`is_resource_type`,
55-
// constructor lowering, etc. Built from the current program only.
55+
// constructor lowering, etc. Built from the current program *and* dependency
56+
// interfaces, so a class/resource/struct defined in another package and
57+
// constructed/held in this source is classified correctly (otherwise it
58+
// falls through to the unknown-type path and mis-lowers, e.g. a named-field
59+
// class constructed as a positional `Widget(1)` call — review #8).
5660
//
57-
// NOTE (review #8): types declared in *dependency* interfaces are not added
58-
// here, so a class/resource/sum defined in another package and constructed
59-
// in this source can mis-lower. A blanket ingest of `interface_programs` is
60-
// *wrong*, though: the bundled stdlib interfaces declare runtime-backed
61+
// Bundled stdlib interface types are EXCLUDED: they declare runtime-backed
6162
// types (e.g. `ProcessRequest`, lowered as `rsscript_runtime::ProcessRequest`)
62-
// as plain structs, and classifying those as local user types reclassifies
63-
// them and drops the `rsscript_runtime::` qualification. The correct fix must
64-
// distinguish runtime-backed stdlib types from genuine dependency types and
65-
// be validated with a multi-package repro — tracked in BUGS.md (RSS-14).
66-
let type_kinds = program
67-
.items
63+
// as plain structs, and classifying those as local user types would drop the
64+
// `rsscript_runtime::` qualification. So only *non-builtin* (dependency)
65+
// interface types are ingested; the current program wins on any conflict.
66+
let builtin_type_names = builtin_interface_type_names();
67+
let type_kinds = interface_programs
6868
.iter()
69+
.flat_map(|interface| interface.items.iter())
70+
.filter(|item| match item {
71+
Item::Type(ty) => !builtin_type_names.contains(ty.name.as_str()),
72+
_ => false,
73+
})
74+
.chain(program.items.iter())
6975
.filter_map(|item| match item {
7076
Item::Type(ty) => Some((ty.name.clone(), ty.kind)),
7177
Item::SumType(_) => None, // sum types don't contribute to type_kinds map
@@ -5456,6 +5462,25 @@ fn builtin_generic_type_params(root: &str) -> Option<Vec<&'static str>> {
54565462
}
54575463
}
54585464

5465+
/// Type names declared by the bundled stdlib (`builtin`) interfaces. These are
5466+
/// runtime-backed (lowered as `rsscript_runtime::X`), so they must be kept out of
5467+
/// the per-package `type_kinds` map even though they appear among the interface
5468+
/// programs — otherwise they would be reclassified as local user types. Parsed
5469+
/// once and cached (the builtin interface set is fixed at compile time).
5470+
fn builtin_interface_type_names() -> &'static std::collections::HashSet<String> {
5471+
static NAMES: std::sync::OnceLock<std::collections::HashSet<String>> =
5472+
std::sync::OnceLock::new();
5473+
NAMES.get_or_init(|| {
5474+
crate::interfaces::builtin_interfaces()
5475+
.flat_map(|(file, source)| crate::syntax::parse_source(file, source).items.into_iter())
5476+
.filter_map(|item| match item {
5477+
Item::Type(ty) => Some(ty.name),
5478+
_ => None,
5479+
})
5480+
.collect()
5481+
})
5482+
}
5483+
54595484
fn fn_type_ref(params: Vec<TypeRef>, return_ty: Option<TypeRef>, span: &Span) -> TypeRef {
54605485
TypeRef {
54615486
name: "Fn".to_string(),

crates/rsscript/tests/checker_lowering/types.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,3 +585,34 @@ fn run() -> Result<Unit, ChannelError> {
585585
"typed Channel<Int> let should emit a concrete Rust annotation, got:\n{lowered}"
586586
);
587587
}
588+
589+
#[test]
590+
fn rust_lowering_classifies_dependency_interface_types() {
591+
// Review #8: a class declared in a *dependency* interface (not the current
592+
// source) must be classified so it lowers correctly. Previously it fell through
593+
// to the unknown-type path and lowered as a positional `Widget(1i64)` call,
594+
// which can't construct a named-field struct/class. Now it lowers as a managed
595+
// named-field construction. Runtime-backed stdlib types are excluded from this
596+
// ingest (covered by the process/config lowering tests staying qualified).
597+
let interfaces = vec![(
598+
"dep/widget.rssi".to_string(),
599+
"class Widget {\n v: Int\n}\n".to_string(),
600+
)];
601+
let sources = vec![(
602+
"main.rss".to_string(),
603+
"fn build() -> Widget {\n return Widget(v: 1)\n}\n\nfn main() -> Unit {\n let w = build()\n return Unit\n}\n".to_string(),
604+
)];
605+
let package =
606+
lower_sources_to_rust_package_with_options(&sources, "dep-types", "/rt", &interfaces, &[])
607+
.expect("dependency-typed source should lower");
608+
assert!(
609+
package.lib_rs.contains("Widget { v: 1i64 }"),
610+
"dependency class should construct a named-field struct, got:\n{}",
611+
package.lib_rs
612+
);
613+
assert!(
614+
!package.lib_rs.contains("Widget(1i64)"),
615+
"dependency class must not lower to a positional tuple-struct call:\n{}",
616+
package.lib_rs
617+
);
618+
}

docs/RSScript_v0.6_Spec.md

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,9 @@ read / mut / take effects
641641
resource values through with
642642
ResourcePool<T: Resource>
643643
bodyless native declarations through package binding metadata
644-
restricted executable async bodies and direct await
644+
async fn bodies: direct await plus statement-boundary awaits in if/loop/match/with
645+
structured concurrency: task_group { async let ... }, select, await for
646+
bounded MPSC Channel and Stream (Receiver.into_stream / Stream.next), via rss-async
645647
receiver-call shorthand with unique resolution
646648
match expressions in expression position
647649
extended collection pipeline (List/Map noescape operations)
@@ -656,20 +658,26 @@ REIR adapter for capability-binding evidence
656658
The following may be parsed and surfaced for review but are not executable lowering targets in v0.6:
657659

658660
```text
659-
spawn
660-
future task runtime
661+
spawn (unstructured task creation)
662+
public Future / Waker / task-handle surface
661663
Rust-style open enum machinery beyond sealed RSScript sum types
662664
general user FFI
663665
advanced protocol/dynamic dispatch model (capability objects)
664-
Stream<T> / await-for
665666
scoped views / slices
666-
compiler-owned derives
667667
```
668668

669-
`async fn` signatures are review-visible contracts. v0.6 admits a restricted
670-
executable async MVP: `await` may appear only inside an `async fn`, and it must
671-
directly consume an async call. RSScript does not expose `Future`, `Pin`, `Poll`,
672-
`Waker`, Rust executor internals, or task handles as source-level types.
669+
`async fn` signatures are review-visible contracts. v0.6 admits an executable async
670+
MVP: `await` appears inside an `async fn`, either directly consuming an async call
671+
at a statement boundary or inside an `if`/`loop`/`match`/`with` body (which lowers
672+
as an explicit async statement boundary); awaits embedded in ordinary expression
673+
arguments are rejected (`RS0411`) until full async expression lowering lands.
674+
Structured concurrency is executable: `task_group { async let ... }` constructs
675+
isolate-local child operations driven by one cooperative poll loop, `select`
676+
awaits the first ready arm, and `await for` iterates a `Stream` / channel
677+
`Receiver`. The user-facing async library (`Channel`, `Stream`, async file/HTTP/
678+
process IO, timers, cancellation) lives in the `rss-async` package. RSScript does
679+
not expose `Future`, `Pin`, `Poll`, `Waker`, Rust executor internals, or public
680+
task handles as source-level types.
673681

674682
The v0.6 execution target is single-isolate and cooperative. `await` is a
675683
suspension boundary in the async function frame: Copy values and managed handles

0 commit comments

Comments
 (0)