From 054231def78dc63addddacb3195fb664d10a11e8 Mon Sep 17 00:00:00 2001 From: onatozmenn Date: Wed, 5 Aug 2026 17:41:33 +0300 Subject: [PATCH] Compile the two shapes the checker accepted and the backend could not lower A pattern inside a record pattern, `Box { size: Inner { depth } }`, and a `let` that takes a value apart, `let Point { x, y } = point`. Both check. Both run under the interpreter. Neither could be lowered, so `deed build` and `deed test --compiled` had nothing to say about a file that used one. Nothing had noticed, and the reason is worth writing down: a program the backend cannot lower and a program with no tests in it produce the same silence. `--compiled` prints "no tests found in the compiled backend" for both. `corpus_backend.rs` already said the backend refuses nothing, and that claim was true. It was a claim about `examples/`, which is the shapes one author happened to write, not about the language. So the fix comes with the ratchet that would have caught it: every case in `conformance/` the checker accepts is now held to being lowerable, and the conformance suite is written the other way round, to cover the language rather than to be a program. Break-verified: reverting the `let` lowering fails `the_backend_lowers_every_conformance_case_the_checker_accepts` by name. Both shapes also join `agreement.rs`, so the two engines are pinned to the same answer rather than only to having one. --- CHANGELOG.md | 15 +++ .../case.txt | 3 + .../program.deed | 21 ++++ .../run-a-pattern-inside-a-pattern/case.txt | 4 + .../program.deed | 28 +++++ crates/deed-driver/tests/agreement.rs | 12 ++ .../deed-driver/tests/conformance_backend.rs | 119 ++++++++++++++++++ crates/deed-driver/tests/corpus_backend.rs | 6 + crates/deed-mir/src/lower.rs | 68 +++++++++- 9 files changed, 271 insertions(+), 5 deletions(-) create mode 100644 conformance/cases/run-a-let-that-takes-a-value-apart/case.txt create mode 100644 conformance/cases/run-a-let-that-takes-a-value-apart/program.deed create mode 100644 conformance/cases/run-a-pattern-inside-a-pattern/case.txt create mode 100644 conformance/cases/run-a-pattern-inside-a-pattern/program.deed create mode 100644 crates/deed-driver/tests/conformance_backend.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 813bfb7e..74deb604 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,21 @@ release notes. the minus in front is the unary one is a question about the grammar, and the parser reads the pair as one number. Anywhere else the digits are still the literal that does not fit, reported once, by the pass that knows. + +- The backend compiles two shapes the checker had always accepted: a pattern + inside a record pattern, `Box { size: Inner { depth } }`, and a `let` that + takes a value apart, `let Point { x, y } = point`. Both checked, both ran + under the interpreter, and neither could be lowered, so `deed build` and + `deed test --compiled` quietly had nothing to say about a file that used + one. Nothing had noticed because a program the backend cannot lower and a + program with no tests in it produce the same silence. + + Every case in `conformance/` that the checker accepts is now held to being + lowerable. The suite that already said the backend refuses nothing was + measuring `examples/`, which is the shapes one author happened to write; the + conformance cases exist to cover the language, which is the question that + was actually being asked. + ### Standard library - `std/ratio` writes contracts. `absolute` promises a number that is not diff --git a/conformance/cases/run-a-let-that-takes-a-value-apart/case.txt b/conformance/cases/run-a-let-that-takes-a-value-apart/case.txt new file mode 100644 index 00000000..966038ac --- /dev/null +++ b/conformance/cases/run-a-let-that-takes-a-value-apart/case.txt @@ -0,0 +1,3 @@ +mode: run +expect: run +stdout: 5 diff --git a/conformance/cases/run-a-let-that-takes-a-value-apart/program.deed b/conformance/cases/run-a-let-that-takes-a-value-apart/program.deed new file mode 100644 index 00000000..62a130ee --- /dev/null +++ b/conformance/cases/run-a-let-that-takes-a-value-apart/program.deed @@ -0,0 +1,21 @@ +module conformance/case_destructuring_let + +record Point { + x: Int, + y: Int, +} + +// A `let` that takes a value apart. It has no second arm, so the pattern has +// to be one that always applies: a record has one shape, and a choice would +// need a `match`. +fn sum(point: Point) -> Int { + let Point { x, y } = point + x + y +} + +fn main(sys: System) -> () + uses + Io.write, +{ + Io.write(sys.console, to_string(sum(Point { x: 2, y: 3 }))) +} diff --git a/conformance/cases/run-a-pattern-inside-a-pattern/case.txt b/conformance/cases/run-a-pattern-inside-a-pattern/case.txt new file mode 100644 index 00000000..f6ab7ebf --- /dev/null +++ b/conformance/cases/run-a-pattern-inside-a-pattern/case.txt @@ -0,0 +1,4 @@ +mode: run +expect: run +stdout: 7 +stdout: 0 diff --git a/conformance/cases/run-a-pattern-inside-a-pattern/program.deed b/conformance/cases/run-a-pattern-inside-a-pattern/program.deed new file mode 100644 index 00000000..76fb878d --- /dev/null +++ b/conformance/cases/run-a-pattern-inside-a-pattern/program.deed @@ -0,0 +1,28 @@ +module conformance/case_nested_pattern + +record Inner { + depth: Int, +} + +choice Shape { + Dot, + Box { size: Inner }, +} + +// A pattern inside a pattern. The outer one chooses the arm and the inner one +// only names what is already there, which is why it needs no condition of its +// own: `Inner` has one shape, so reaching into it always applies. +fn depth_of(shape: Shape) -> Int { + match shape { + Dot => 0, + Box { size: Inner { depth } } => depth, + } +} + +fn main(sys: System) -> () + uses + Io.write, +{ + Io.write(sys.console, to_string(depth_of(Box { size: Inner { depth: 7 } }))) + Io.write(sys.console, to_string(depth_of(Dot))) +} diff --git a/crates/deed-driver/tests/agreement.rs b/crates/deed-driver/tests/agreement.rs index 5fb2d1c3..6874190a 100644 --- a/crates/deed-driver/tests/agreement.rs +++ b/crates/deed-driver/tests/agreement.rs @@ -61,6 +61,18 @@ fn programs() -> Vec { call: "answer", expect: 12, }, + Agreed { + name: "a pattern inside a pattern", + source: "module a\n\nrecord Inner {\n depth: Int,\n}\n\nchoice Shape {\n Dot,\n Box { size: Inner },\n}\n\nfn depth_of(shape: Shape) -> Int {\n match shape {\n Dot => 0,\n Box { size: Inner { depth } } => depth,\n }\n}\n\nfn answer() -> Int { depth_of(Box { size: Inner { depth: 7 } }) + depth_of(Dot) }\n\ntest \"the inner pattern names what the outer one reached\" {\n assert answer() == 7\n}\n", + call: "answer", + expect: 7, + }, + Agreed { + name: "a let that takes a value apart", + source: "module a\n\nrecord Point {\n x: Int,\n y: Int,\n}\n\nfn sum(point: Point) -> Int {\n let Point { x, y } = point\n x + y\n}\n\nfn answer() -> Int { sum(Point { x: 2, y: 3 }) }\n\ntest \"a let can name the fields\" {\n assert answer() == 5\n}\n", + call: "answer", + expect: 5, + }, Agreed { name: "a return inside an if", source: "module a\n\nfn absolute(n: Int) -> Int {\n if n >= 0 {\n return n\n }\n 0 - n\n}\n\nfn answer() -> Int { absolute(5) + absolute(2 - 7) }\n\ntest \"one branch can return early\" {\n assert answer() == 10\n}\n", diff --git a/crates/deed-driver/tests/conformance_backend.rs b/crates/deed-driver/tests/conformance_backend.rs new file mode 100644 index 00000000..df4f4447 --- /dev/null +++ b/crates/deed-driver/tests/conformance_backend.rs @@ -0,0 +1,119 @@ +//! Every shape the conformance suite accepts, the backend compiles. +//! +//! `crates/deed-driver/tests/corpus_backend.rs` already says the backend +//! refuses nothing in `examples/`, and that turned out to be a claim about the +//! corpus rather than about the language: a corpus is the shapes one author +//! happened to write. Two that nobody had written, a pattern inside a record +//! pattern and a `let` that takes a value apart, checked cleanly, ran under the +//! interpreter, and could not be lowered at all. Nothing said so, because a +//! test skipped for want of a compiled body looks exactly like a file with no +//! tests in it. +//! +//! `conformance/` is the other corpus, and it is written the other way round: +//! its cases exist to cover the language rather than to be a program. So this +//! holds every case the suite expects to check or to run to being lowerable, +//! which is the question `corpus_backend.rs` asks of `examples/`. +//! +//! Lowering rather than answering. What the two engines answer is +//! `agreement.rs`, which needs a program written to be compared; this needs +//! only that the backend has something to say at all. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +use deed_diagnostics::SourceMap; +use deed_driver::{check_all, shipped_for, shipped_source}; + +fn root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .expect("the crate sits two levels under the repository root") + .to_path_buf() +} + +/// Every case whose program is meant to be accepted, by directory name. +/// +/// A `reject` case is a program the checker turns away, so the backend never +/// sees one and holding it to anything would be holding it to a program that +/// does not exist. +fn accepted() -> Vec<(String, String)> { + let cases = root().join("conformance").join("cases"); + let mut names: BTreeSet = BTreeSet::new(); + for entry in std::fs::read_dir(&cases).expect("conformance/cases should be there") { + let path = entry.expect("a readable entry").path(); + if path.is_dir() { + names.insert(path); + } + } + + let mut found = Vec::new(); + for dir in names { + let name = dir + .file_name() + .expect("a directory has a name") + .to_string_lossy() + .to_string(); + if name.starts_with("reject-") { + continue; + } + let program = dir.join("program.deed"); + if let Ok(text) = std::fs::read_to_string(&program) { + found.push((name, text)); + } + } + + assert!( + found.len() > 10, + "the suite should carry more than {} cases the checker accepts", + found.len() + ); + found +} + +#[test] +fn the_backend_lowers_every_conformance_case_the_checker_accepts() { + let mut refused = Vec::new(); + + for (name, text) in accepted() { + let mut sources = SourceMap::new(); + let subject = sources.add(format!("{name}/program.deed"), text.clone()); + let mut ids = vec![subject]; + for module in shipped_for([text.as_str()]) { + let source = shipped_source(module).expect("a module that ships has a source"); + ids.push(sources.add(format!("{module}.deed"), source.to_string())); + } + + let checks = check_all(&sources, &ids); + assert!( + !checks[0].has_errors(), + "`{name}` is an accept case and should check" + ); + + let alongside: Vec> = checks[1..] + .iter() + .map(|checked| deed_mir::Alongside { + module: &checked.module, + resolutions: &checked.resolutions, + types: &checked.types, + }) + .collect(); + + if let Err(why) = deed_mir::lower_with_tests_alongside( + &checks[0].module, + &checks[0].resolutions, + &checks[0].types, + &alongside, + ) { + refused.push(format!("{name}: {why}")); + } + } + + assert!( + refused.is_empty(), + "the backend compiles every shape the conformance suite accepts, and now refuses \ + {}:\n{}", + refused.len(), + refused.join("\n") + ); +} diff --git a/crates/deed-driver/tests/corpus_backend.rs b/crates/deed-driver/tests/corpus_backend.rs index 92e3b27b..baad2063 100644 --- a/crates/deed-driver/tests/corpus_backend.rs +++ b/crates/deed-driver/tests/corpus_backend.rs @@ -12,6 +12,12 @@ //! backend compiles a subset of the language on purpose and the number below //! is a floor rather than a goal, so that a change which quietly stops //! compiling half the corpus is loud. +//! +//! What this does not say is that the backend compiles the language. A corpus +//! is the shapes one author happened to write, and twice now a shape nobody +//! had written checked, ran, and could not be lowered. The question about the +//! language is asked in `conformance_backend.rs`, over the suite that exists +//! to cover it. use std::path::{Path, PathBuf}; diff --git a/crates/deed-mir/src/lower.rs b/crates/deed-mir/src/lower.rs index 46f134b3..a42a748c 100644 --- a/crates/deed-mir/src/lower.rs +++ b/crates/deed-mir/src/lower.rs @@ -1995,17 +1995,54 @@ impl Lowering<'_> { } => { let value = self.expr(init)?; let ty = self.ty_at(init.span())?; - let local = self.function.add_local(ty); + let local = self.function.add_local(ty.clone()); match pattern { ast::Pattern::Path { segments, .. } if segments.len() == 1 => { if let Some(def) = self.resolutions.resolution(segments[0].span) { self.slots.insert(def, local); } + stmts.push(Stmt::Assign { local, value }); + } + ast::Pattern::Wildcard(_) => stmts.push(Stmt::Assign { local, value }), + // A `let` has no second arm, so the pattern has to + // always apply. The checker already refuses one that + // does not, and this reads the same paths a match arm + // reads rather than a second way of taking a value + // apart. + other => { + let Ty::Aggregate(layout) = ty else { + return Err(unlowered("a `let` that takes a value apart", *span)); + }; + let (named, bindings) = self.arm_pattern(other, layout)?; + if named.len() != self.layout(layout).variants.len() { + return Err(unlowered("a `let` that could fail", *span)); + } + stmts.push(Stmt::Assign { local, value }); + for (path, name) in bindings { + let mut read = Expr::Local(local); + let mut field_ty = Ty::Unit; + for (layout, variant, field) in path { + field_ty = self.layout(layout).variants[variant].fields[field] + .ty + .clone(); + read = Expr::Field { + value: Box::new(read), + layout, + variant, + field, + }; + } + let bound = self.function.add_local(field_ty); + if let Some(def) = self.resolutions.resolution(name.span) { + self.slots.insert(def, bound); + } + stmts.push(Stmt::Assign { + local: bound, + value: read, + }); + } } - ast::Pattern::Wildcard(_) => {} - _ => return Err(unlowered("a `let` that takes a value apart", *span)), } - stmts.push(Stmt::Assign { local, value }); } ast::Stmt::Expr(expr) => { let value = self.expr(expr)?; @@ -3341,8 +3378,29 @@ impl Lowering<'_> { .last() .ok_or_else(|| unlowered("an empty pattern", *span))?, Some(ast::Pattern::Wildcard(_)) => continue, + // One level further in, which is the same read twice + // over. The inner pattern has to always apply, for + // the reason the one inside `err(..)` does: an arm + // has one condition and a pattern that could fail + // would need a second. Some(other) => { - return Err(unlowered("a pattern inside a pattern", other.span())); + let field_ty = held.variants[at].fields[index].ty.clone(); + let Ty::Aggregate(inner) = field_ty else { + return Err(unlowered("a pattern inside a pattern", other.span())); + }; + let (named, held_by) = self.arm_pattern(other, inner)?; + if named.len() != self.layout(inner).variants.len() { + return Err(unlowered( + "a pattern inside a pattern that has to be tested", + other.span(), + )); + } + for (path, name) in held_by { + let mut reached = vec![(layout, at, index)]; + reached.extend(path); + bindings.push((reached, name)); + } + continue; } }; bindings.push((vec![(layout, at, index)], bound));