From 224a6395bd97c59a0495eb4334efe6e79a170d8e Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 10 Sep 2026 21:44:25 +0000 Subject: [PATCH 1/7] ruff_cpp_spo: capture free-function signatures, and wire the C arm end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The C-library arm could name a function but not its signature: CppFunction carried namespace/name/calls only, while the class plane's CppMethod already carried return_type/param_types/is_static. So a downstream MethodSig manifest built from a C corpus would have held an empty parameter list and ret: None for every entry — a signature plane with no signatures. This captures the three missing facts in collect_functions, using the SAME idiom the class walker already uses (void filtered out, types verbatim from the cursor) so the two planes agree and one manifest can read both. No new predicate is needed: returns_type / has_param_type / is_static already exist in the closed vocabulary as the AST-DLL signature shape, so the ontology gate is untouched. File-scope `static` is read from the storage class, not is_static_method(): in C it means internal linkage, and is_static_method() is false for every free function, so reading the member flag would have made the field constant-false. The paired test asserts both arms so the flag has to carry information. Also adds examples/harvest_openggs.rs — the arm wired from libclang to rendered Rust on a real C corpus (OpenGGS, 55 TUs of plain C: 25 file-scope structs, 181 free functions, no classes, so the class plane harvests nothing from it). A translation unit is the Model, because that is what the C programmer used for grouping and for internal linkage. Measured: 55 TUs, 0 parse failures, 181 functions, 763 triples, 527 on the signature plane. The round-trip oracle gates the run: decompile(project(g)) must equal expand(g) restricted to the signature plane, computed by two different code paths, and a mismatch writes nothing. It earned its place on the first run by catching a real bug — the graph was built with ModelGraph::default(), leaving the namespace empty, so every manifest IRI was keyed differently from the expanded triples (527 lost, 527 invented). A self-golden check would have passed. What this generates is signature manifests, not bodies. That is the 85/15 split: the mechanical structure that classifies and orders a transcode is minted; the function bodies remain the essential-15% hand-port. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- crates/ruff_cpp_spo/Cargo.toml | 4 + .../ruff_cpp_spo/examples/harvest_openggs.rs | 195 ++++++++++++++++++ crates/ruff_cpp_spo/src/clang_walker.rs | 101 +++++++++ crates/ruff_cpp_spo/src/lib.rs | 26 +++ 4 files changed, 326 insertions(+) create mode 100644 crates/ruff_cpp_spo/examples/harvest_openggs.rs diff --git a/crates/ruff_cpp_spo/Cargo.toml b/crates/ruff_cpp_spo/Cargo.toml index 8b4c4d08f52e0..63ef98084bc58 100644 --- a/crates/ruff_cpp_spo/Cargo.toml +++ b/crates/ruff_cpp_spo/Cargo.toml @@ -66,6 +66,10 @@ ruff_cpp_codegen = { workspace = true } [lints] workspace = true +[[example]] +name = "harvest_openggs" +required-features = ["libclang"] + [[example]] name = "harvest_events" required-features = ["libclang"] diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs new file mode 100644 index 0000000000000..8f2e1cd784914 --- /dev/null +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -0,0 +1,195 @@ +//! THE `OpenGGS` signature harvest — the C-library arm wired end to end, from +//! libclang to rendered Rust, with the round-trip oracle as the gate. +//! +//! `OpenGGS` () is plain C: 25 file-scope +//! structs and ~181 free functions, no classes. So the class plane +//! (`walk_tu` → `CppClass` → `model_from_class`) harvests NOTHING from it, and +//! the C-library arm ([`walk_free_functions`]) is the whole story. +//! +//! ```text +//! OpenGGS/src/*.cpp --(walk_free_functions)--> CppFunction +//! --(one Model per TU)--> ModelGraph +//! --(expand)--> Vec the SPO plane +//! --(ruff_cpp_codegen::project)--> ClassManifest +//! --(decompile ∘ project == expand | signature)--> THE ORACLE +//! --(render)--> Rust `&[MethodSig]` const tables +//! ``` +//! +//! # A translation unit is the Model +//! +//! `project` keys manifests by `Model::name`, and a C library has no class to +//! key on. The translation unit is the honest unit: it is what the C programmer +//! used for grouping and for internal linkage (`static` = TU-private), so +//! `GAME_ENVIRONMENT.cpp`'s functions become the `GAME_ENVIRONMENT` manifest. +//! Inventing one synthetic class for the whole corpus would throw that grouping +//! away; inventing one per function would make the manifest layer meaningless. +//! +//! # What this generates, and what it does NOT +//! +//! Signature manifests — names, ordered parameter types, return types, linkage. +//! **Not bodies.** That is the 85/15 split: this mints the mechanical structure +//! that classifies and orders the transcode, and the function bodies remain the +//! essential-15% hand-port. A generated `MethodSig` table is a map, not a port. +//! +//! Env: +//! `GGS_SRC` corpus directory (default `/home/user/OpenGGS/src`) +//! `GGS_ARGS` extra clang args, space-separated +//! (default `-I/usr/include/SDL2`) +//! `GGS_OUT` artifact directory (default `.claude/harvest/openggs`) +//! +//! A missing corpus is a printed skip, never a failure, so this runs on any +//! checkout. The corpus is read locally and never redistributed. +//! +//! ```sh +//! LIBCLANG_PATH=/usr/lib/llvm-18/lib \ +//! cargo run -p ruff_cpp_spo --features libclang --example harvest_openggs +//! ``` + +#![expect( + clippy::print_stderr, + reason = "manifest-emission CLI example (mirrors harvest_ladybug)" +)] + +use std::collections::BTreeSet; +use std::path::PathBuf; + +use ruff_cpp_spo::{NAMESPACE, walk_free_functions}; +use ruff_spo_triplet::{CppMethod, Model, ModelGraph, Triple, expand, to_ndjson}; + +fn env_or(key: &str, default: &str) -> String { + std::env::var(key).unwrap_or_else(|_| default.to_string()) +} + +/// The translation units of the corpus, sorted so the harvest is reproducible. +fn translation_units(src: &PathBuf) -> std::io::Result> { + let mut out: Vec = std::fs::read_dir(src)? + .filter_map(Result::ok) + .map(|e| e.path()) + .filter(|p| p.extension().is_some_and(|e| e == "cpp")) + .collect(); + out.sort(); + Ok(out) +} + +fn main() -> Result<(), Box> { + let src = PathBuf::from(env_or("GGS_SRC", "/home/user/OpenGGS/src")); + if !src.is_dir() { + eprintln!("[ggs] skip: no corpus at {}", src.display()); + return Ok(()); + } + let mut args: Vec = env_or("GGS_ARGS", "-I/usr/include/SDL2") + .split_whitespace() + .map(str::to_string) + .collect(); + // The corpus's own headers are beside its sources; every TU includes + // `globals.h` by bare name. + args.push(format!("-I{}", src.display())); + + let units = translation_units(&src)?; + // The namespace lives on the GRAPH, not the model, and `decompile` is told + // the same one below. Building the graph with `default()` leaves it empty, + // which keys every manifest IRI differently from the expanded triples — the + // oracle caught exactly that on the first run. + let mut graph = ModelGraph::new(NAMESPACE); + let (mut n_functions, mut n_failed) = (0usize, 0usize); + + for tu in &units { + let stem = tu + .file_stem() + .map_or_else(String::new, |s| s.to_string_lossy().into_owned()); + let funcs = match walk_free_functions(tu, &args) { + Ok(f) => f, + Err(e) => { + eprintln!("[ggs] {stem}: parse failed: {e}"); + n_failed += 1; + continue; + } + }; + if funcs.is_empty() { + continue; + } + // A function DEFINED in a header reachable from several TUs is harvested + // once per TU. Keeping the first sighting per name inside one model is a + // corpus decision, not a dedup of occurrences. + let mut seen = BTreeSet::new(); + let mut model = Model::new(&stem); + for f in funcs { + if !seen.insert(f.name.clone()) { + continue; + } + n_functions += 1; + model.methods.push(CppMethod { + name: f.name, + return_type: f.return_type, + param_types: f.param_types, + is_static: f.is_static, + ..CppMethod::default() + }); + } + graph.models.push(model); + } + + let triples = expand(&graph); + + // ── THE ORACLE ─────────────────────────────────────────────────────────── + // `decompile(project(g))` must equal `expand(g)` restricted to the + // signature plane. This is not a self-golden: the two sides are computed by + // different code (the expander vs the manifest projection), so a manifest + // that drops a method or reorders a parameter list fails here. A run that + // cannot prove it refuses to write anything. + let manifests = ruff_cpp_codegen::project(&graph); + let round_tripped: BTreeSet = ruff_cpp_codegen::decompile(&manifests, NAMESPACE) + .iter() + .map(key) + .collect(); + let expected: BTreeSet = triples + .iter() + .filter(|t| ruff_cpp_codegen::is_signature_plane(t)) + .map(key) + .collect(); + + let missing: Vec<&String> = expected.difference(&round_tripped).collect(); + let extra: Vec<&String> = round_tripped.difference(&expected).collect(); + if !missing.is_empty() || !extra.is_empty() { + eprintln!( + "[ggs] ORACLE FAILED: {} signature triples lost, {} invented", + missing.len(), + extra.len() + ); + for k in missing.iter().take(5) { + eprintln!(" lost: {k}"); + } + for k in extra.iter().take(5) { + eprintln!(" invented: {k}"); + } + return Err("signature round-trip mismatch — nothing written".into()); + } + + let out = PathBuf::from(env_or("GGS_OUT", ".claude/harvest/openggs")); + std::fs::create_dir_all(&out)?; + let rendered = ruff_cpp_codegen::render(&manifests); + std::fs::write(out.join("openggs_methods.rs"), &rendered)?; + + std::fs::write(out.join("triples.ndjson"), to_ndjson(&triples))?; + + eprintln!( + "[ggs] {} TUs ({n_failed} failed to parse), {} models, {n_functions} functions", + units.len(), + graph.models.len() + ); + eprintln!( + "[ggs] {} triples ({} on the signature plane)", + triples.len(), + expected.len() + ); + eprintln!("[ggs] ORACLE PASSED: decompile(project(g)) == expand(g) | signature plane"); + eprintln!("[ggs] artifacts -> {}", out.display()); + Ok(()) +} + +/// A triple's identity for set comparison. Provenance is deliberately excluded: +/// the oracle is about WHICH facts survive the projection, not about how each +/// side labelled its own derivation. +fn key(t: &Triple) -> String { + format!("{}\t{}\t{}", t.s, t.p, t.o) +} diff --git a/crates/ruff_cpp_spo/src/clang_walker.rs b/crates/ruff_cpp_spo/src/clang_walker.rs index 93619bad9ca25..d3cd17b77a4f4 100644 --- a/crates/ruff_cpp_spo/src/clang_walker.rs +++ b/crates/ruff_cpp_spo/src/clang_walker.rs @@ -42,6 +42,7 @@ use std::path::Path; use clang::diagnostic::Severity; use clang::{ Accessibility, Clang, Entity, EntityKind, ExceptionSpecification, Index, RefQualifier, + StorageClass, }; use ruff_spo_triplet::{ CppAccess, CppBase, CppField, CppFriend, CppMethod, CppRefQualifier, CppTemplate, @@ -260,10 +261,32 @@ fn collect_functions(entity: &Entity, out: &mut Vec) { collect_calls(&child, &mut calls); calls.sort(); calls.dedup(); + // AST-DLL signature shape, captured exactly as the class + // walker captures it for a member (`void` filtered out, types + // verbatim from the cursor) so the two planes agree and one + // downstream manifest can read both. + let return_type = child + .get_result_type() + .map(|t| t.get_display_name()) + .filter(|d| !d.is_empty() && d != "void"); + let param_types = child + .get_arguments() + .into_iter() + .flatten() + .filter_map(|a| a.get_type().map(|t| t.get_display_name())) + .collect(); + // File-scope `static` is INTERNAL LINKAGE in C, not a + // class-level member. `is_static_method()` answers the + // member question and is false for a free function, so the + // storage class is the right source here. + let is_static = child.get_storage_class() == Some(StorageClass::Static); out.push(CppFunction { namespace: enclosing_scopes(&child), name, calls, + return_type, + param_types, + is_static, }); } } @@ -2419,6 +2442,84 @@ int root(int x) { return leaf(x); } assert_eq!(root.calls, vec!["leaf".to_string()]); } + /// The AST-DLL signature shape for FREE functions: without these three + /// fields the C-library arm can name a function but not its signature, so a + /// downstream `MethodSig` manifest carries an empty parameter list for every + /// entry. + const FREE_FN_SIGNATURE_SRC: &str = r" +int with_params(const char *name, int count, double scale) { return count; } +void returns_nothing(int a) { (void)a; } +static int tu_private(void) { return 1; } +const char *returns_pointer(void) { return nullptr; } +"; + + #[test] + fn free_function_signatures_are_captured_in_order() { + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("free_fn_signature", FREE_FN_SIGNATURE_SRC); + let funcs = walk_free_functions(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + + let find = |n: &str| { + funcs + .iter() + .find(|f| f.name == n) + .unwrap_or_else(|| panic!("{n} missing; got {funcs:?}")) + }; + + let with_params = find("with_params"); + // Order is load-bearing: the expander encodes position in the object, + // so a reordered vector silently produces a different signature. A + // set-equality assertion here would not catch that. + assert_eq!( + with_params.param_types, + vec![ + "const char *".to_string(), + "int".to_string(), + "double".to_string() + ] + ); + assert_eq!(with_params.return_type.as_deref(), Some("int")); + + // `void` is ABSENT, not `Some("void")` — the AST-DLL shape reads a + // missing `returns_type` as "no value returned". + let nothing = find("returns_nothing"); + assert_eq!(nothing.return_type, None); + assert_eq!(nothing.param_types, vec!["int".to_string()]); + + // A `(void)` parameter list is zero parameters, not one named `void`. + let ptr = find("returns_pointer"); + assert!(ptr.param_types.is_empty(), "params {:?}", ptr.param_types); + assert_eq!(ptr.return_type.as_deref(), Some("const char *")); + } + + #[test] + fn file_scope_static_is_reported_as_internal_linkage() { + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("free_fn_static", FREE_FN_SIGNATURE_SRC); + let funcs = walk_free_functions(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + + let find = |n: &str| { + funcs + .iter() + .find(|f| f.name == n) + .unwrap_or_else(|| panic!("{n} missing; got {funcs:?}")) + }; + // The discriminating pair: `is_static_method()` is false for BOTH of + // these (neither is a member), so reading the storage class is what + // makes the flag carry information rather than being constant false. + assert!(find("tu_private").is_static, "file-scope static missed"); + assert!( + !find("with_params").is_static, + "external linkage misreported" + ); + } + /// Fix 3 (walker half): a TU with an unresolved `#include` still parses /// (`walk_tu` alone returns `Ok`, "0 failed") but /// [`walk_tu_with_diagnostics`] surfaces the severity>=Error diagnostic a diff --git a/crates/ruff_cpp_spo/src/lib.rs b/crates/ruff_cpp_spo/src/lib.rs index 62e205f1b2f76..73d3d55a22ab2 100644 --- a/crates/ruff_cpp_spo/src/lib.rs +++ b/crates/ruff_cpp_spo/src/lib.rs @@ -148,6 +148,32 @@ pub struct CppFunction { /// graph. Distinct from the AR/OO `BodyArm.calls` (persistence mutators /// only); this is EVERY `CallExpr` callee. pub calls: Vec, + /// Return type, verbatim (e.g. `bool`, `const char *`). `None` (and not + /// emitted) for `void` — the AST-DLL shape reads an absent `returns_type` + /// as "no value returned". + /// + /// Same capture as [`CppMethod::return_type`], and deliberately the same + /// closed-vocab predicate (`returns_type`): a free function's signature is + /// not a different KIND of fact from a member's, so it needs no new + /// predicate and no second encoding. + pub return_type: Option, + /// Parameter types in signature order, verbatim — one `has_param_type` + /// each, positions riding the `:` object encoding exactly as + /// [`CppMethod::param_types`] does. + /// + /// Without this the C-library arm could name a function but not its + /// signature, so a downstream `MethodSig` manifest would carry an empty + /// parameter list for every entry — a signature plane with no signatures. + pub param_types: Vec, + /// `static` at file scope — internal linkage, i.e. NOT part of the + /// library's API surface. + /// + /// This is the C meaning of `static` (translation-unit-private), which is a + /// different fact from [`CppMethod::is_static`]'s class-level member. It + /// rides the same `is_static` predicate because both answer "no implicit + /// receiver, not an instance-bound call"; the distinction that matters to a + /// transcode — is this callable from outside the TU — is the one captured. + pub is_static: bool, } /// A C or C++ enum declaration, with fully-explicit variant values. From 4dd267a4d7ba363a4a5ded08f21e5f804f8d3c57 Mon Sep 17 00:00:00 2001 From: Jan Date: Fri, 11 Sep 2026 03:51:35 +0000 Subject: [PATCH 2/7] Reject translation units with error diagnostics in the C-library arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit finding, verified real: walk_free_functions parsed a TU and returned Ok without inspecting tu.get_diagnostics(), so an unresolved #include produced an incomplete function model with no Err and no marker. libclang recovers from a missing header by DROPPING the declarations that needed it; the functions are simply absent. The sharp half of the finding is what it says about the oracle. The round-trip check compares decompile(project(g)) against expand(g) — both derived from the SAME graph — so when a parse silently drops functions, both sides are equally short and the oracle passes. It proves the projection faithful; it never proved the harvest complete, and the PR description let "computed by two different code paths" imply more than that. The class plane already had this guard and its doc already described this exact failure mode, with the worked Tesseract STATS/scrollview.h case and the warning that "0 failed" does not mean the sweep is complete. It sits thirty lines above the function used here. The mechanism existed; it was not reached for. - walk_free_functions_with_diagnostics mirrors walk_tu_with_diagnostics: one parse, functions plus every diagnostic at Severity::Error or above. walk_free_functions becomes the thin wrapper, matching how walk_tu wraps its own diagnostics-aware form. - harvest_openggs skips any TU with an error diagnostic and reports the count separately from parse failures, rather than minting a manifest from a partial parse. Measured on the corpus before and after: 55 TUs, 0 failed to parse, 0 skipped on error diagnostics, 181 functions, 763 triples, 527 on the signature plane. So the defect was latent here — the numbers already published are unaffected — and this closes the hole rather than fixing a wrong result. Two tests, because a guard that has never been shown to fire carries no information and one that fires on everything carries none either: an unresolvable include must surface a "file not found" diagnostic while the parse still returns Ok, and a clean TU must report none while still harvesting its functions. Also documents main's two failure postures, which the docstring-coverage check wanted: a missing corpus is a skip, a failed oracle is a hard error that writes nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- .../ruff_cpp_spo/examples/harvest_openggs.rs | 38 +++++++- crates/ruff_cpp_spo/src/clang_walker.rs | 94 ++++++++++++++++++- crates/ruff_cpp_spo/src/lib.rs | 3 +- 3 files changed, 128 insertions(+), 7 deletions(-) diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index 8f2e1cd784914..c0b5a62f1c3f1 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -53,7 +53,7 @@ use std::collections::BTreeSet; use std::path::PathBuf; -use ruff_cpp_spo::{NAMESPACE, walk_free_functions}; +use ruff_cpp_spo::{NAMESPACE, walk_free_functions_with_diagnostics}; use ruff_spo_triplet::{CppMethod, Model, ModelGraph, Triple, expand, to_ndjson}; fn env_or(key: &str, default: &str) -> String { @@ -71,6 +71,14 @@ fn translation_units(src: &PathBuf) -> std::io::Result> { Ok(out) } +/// Run the harvest. +/// +/// Two failure postures, deliberately different: a MISSING corpus is a printed +/// skip and `Ok`, so this example runs on any checkout; a FAILED ORACLE is a +/// hard error that writes nothing, because a manifest whose IRIs disagree with +/// the triples is worse than no manifest. A translation unit that parses but +/// emits an error diagnostic is skipped rather than harvested — see the gate in +/// the loop for why the oracle cannot catch that case. fn main() -> Result<(), Box> { let src = PathBuf::from(env_or("GGS_SRC", "/home/user/OpenGGS/src")); if !src.is_dir() { @@ -91,20 +99,39 @@ fn main() -> Result<(), Box> { // which keys every manifest IRI differently from the expanded triples — the // oracle caught exactly that on the first run. let mut graph = ModelGraph::new(NAMESPACE); - let (mut n_functions, mut n_failed) = (0usize, 0usize); + let (mut n_functions, mut n_failed, mut n_diagnostics) = (0usize, 0usize, 0usize); for tu in &units { let stem = tu .file_stem() .map_or_else(String::new, |s| s.to_string_lossy().into_owned()); - let funcs = match walk_free_functions(tu, &args) { - Ok(f) => f, + let (funcs, diagnostics) = match walk_free_functions_with_diagnostics(tu, &args) { + Ok(fd) => fd, Err(e) => { eprintln!("[ggs] {stem}: parse failed: {e}"); n_failed += 1; continue; } }; + // A TU that parsed but emitted an error diagnostic is NOT usable ore. + // libclang recovers from an unresolved #include by dropping the + // declarations that needed it, with no Err and no marker — so the + // functions are simply absent, and the round-trip oracle below cannot + // see the loss (it compares a projection of the graph against an + // expansion of the SAME graph; both are equally short). Refuse the unit + // rather than mint a manifest from a partial parse. + if !diagnostics.is_empty() { + eprintln!( + "[ggs] {stem}: SKIPPED — {} error diagnostic(s); first: {}", + diagnostics.len(), + diagnostics[0] + ); + for d in diagnostics.iter().skip(1).take(2) { + eprintln!("[ggs] also: {d}"); + } + n_diagnostics += 1; + continue; + } if funcs.is_empty() { continue; } @@ -173,7 +200,8 @@ fn main() -> Result<(), Box> { std::fs::write(out.join("triples.ndjson"), to_ndjson(&triples))?; eprintln!( - "[ggs] {} TUs ({n_failed} failed to parse), {} models, {n_functions} functions", + "[ggs] {} TUs ({n_failed} failed to parse, {n_diagnostics} skipped on error \ + diagnostics), {} models, {n_functions} functions", units.len(), graph.models.len() ); diff --git a/crates/ruff_cpp_spo/src/clang_walker.rs b/crates/ruff_cpp_spo/src/clang_walker.rs index d3cd17b77a4f4..91190cf4e4ace 100644 --- a/crates/ruff_cpp_spo/src/clang_walker.rs +++ b/crates/ruff_cpp_spo/src/clang_walker.rs @@ -198,6 +198,37 @@ pub fn walk_tu_configured( /// [`WalkError::Parse`] if the TU fails to parse. #[cfg(feature = "libclang")] pub fn walk_free_functions(path: &Path, args: &[String]) -> Result, WalkError> { + walk_free_functions_with_diagnostics(path, args).map(|(functions, _)| functions) +} + +/// Like [`walk_free_functions`], but also returns every libclang parse +/// diagnostic at [`Severity::Error`] or higher (one parse, not two — +/// [`walk_free_functions`] is a thin wrapper over this). +/// +/// This is the free-function twin of [`walk_tu_with_diagnostics`], and it +/// exists for the same reason, which that function's doc states for the class +/// plane: libclang recovers from an unresolved `#include` by treating the file +/// as successfully parsed while simply DROPPING the declarations that needed +/// the missing header. No `Err`, no marker — the functions are just ABSENT. +/// +/// The consequence is sharper for a harvest than for a single lookup, and +/// sharper still when a round-trip oracle is watching: an oracle that compares +/// a projection of the graph against an expansion of the SAME graph is blind to +/// this, because both sides are equally missing whatever the parse dropped. It +/// proves the projection faithful, never the harvest complete. A caller +/// sweeping a corpus must call this and refuse the translation unit when the +/// list is non-empty — "0 failed to parse" does NOT mean "the corpus was +/// captured". +/// +/// # Errors +/// +/// [`WalkError::Libclang`] if libclang fails to initialise; +/// [`WalkError::Parse`] if the TU fails to parse at all. +#[cfg(feature = "libclang")] +pub fn walk_free_functions_with_diagnostics( + path: &Path, + args: &[String], +) -> Result<(Vec, Vec), WalkError> { let clang = Clang::new().map_err(WalkError::Libclang)?; let index = Index::new(&clang, false, false); let tu = index @@ -207,9 +238,18 @@ pub fn walk_free_functions(path: &Path, args: &[String]) -> Result= Severity::Error) + .map(|d| ParseDiagnostic { + message: d.formatter().format(), + }) + .collect(); + let mut out = Vec::new(); collect_functions(&tu.get_entity(), &mut out); - Ok(out) + Ok((out, diagnostics)) } /// Recurse the AST, emitting a [`CppFunction`] for every free-function @@ -2442,6 +2482,58 @@ int root(int x) { return leaf(x); } assert_eq!(root.calls, vec!["leaf".to_string()]); } + /// A TU whose `#include` cannot be resolved. libclang parses it + /// "successfully" and DROPS the declaration that needed the header. + const UNRESOLVED_INCLUDE_SRC: &str = r" +#include +int survives(int x) { return x; } +"; + + #[test] + fn an_unresolved_include_is_reported_as_an_error_diagnostic() { + // The can-it-fire half. A guard that has never been shown to trigger + // carries no information, and this one did not exist at all until + // CodeRabbit pointed out that `walk_free_functions` returned Ok on a + // partial parse. + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("unresolved_include", UNRESOLVED_INCLUDE_SRC); + let (funcs, diagnostics) = + walk_free_functions_with_diagnostics(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + + assert!( + !diagnostics.is_empty(), + "an unresolvable include must surface as an error diagnostic; got {funcs:?}" + ); + // And the point of the guard: the parse still returned Ok, so a caller + // that only checks the Result sees nothing wrong. + assert!( + diagnostics + .iter() + .any(|d| d.to_string().contains("file not found")), + "diagnostics {diagnostics:?}" + ); + } + + #[test] + fn a_clean_translation_unit_reports_no_error_diagnostics() { + // The can-it-stay-silent half. Without this, a guard that flagged every + // TU would pass the test above while making the harvest reject + // everything — the same information content as never firing. + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("clean_tu_diags", PLAIN_FREE_FUNCTION_SRC); + let (funcs, diagnostics) = + walk_free_functions_with_diagnostics(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + + assert!(diagnostics.is_empty(), "clean TU reported {diagnostics:?}"); + assert_eq!(funcs.len(), 2, "and still harvests its functions"); + } + /// The AST-DLL signature shape for FREE functions: without these three /// fields the C-library arm can name a function but not its signature, so a /// downstream `MethodSig` manifest carries an empty parameter list for every diff --git a/crates/ruff_cpp_spo/src/lib.rs b/crates/ruff_cpp_spo/src/lib.rs index 73d3d55a22ab2..3ecc7de0b61bc 100644 --- a/crates/ruff_cpp_spo/src/lib.rs +++ b/crates/ruff_cpp_spo/src/lib.rs @@ -74,7 +74,8 @@ pub mod events; #[cfg(feature = "libclang")] pub use clang_walker::{ BodyArmConfig, MAPPED_CURSOR_KINDS, ParseDiagnostic, WalkError, class_body_cursor_histogram, - walk_enums, walk_free_functions, walk_tu, walk_tu_configured, walk_tu_with_diagnostics, + walk_enums, walk_free_functions, walk_free_functions_with_diagnostics, walk_tu, + walk_tu_configured, walk_tu_with_diagnostics, }; /// The namespace prefix for C++ machine-plane subjects/objects. From 5fb478585181b5c25681306e7f97008777c4a07b Mon Sep 17 00:00:00 2001 From: Jan Date: Fri, 11 Sep 2026 03:55:07 +0000 Subject: [PATCH 3/7] Stop overloading is_static with C internal linkage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second codex finding, verified real. The example mapped CppFunction::is_static (C file-scope linkage) onto CppMethod::is_static, but that predicate means "a class-level member with no implicit this". A free function is not a class member at all, so neither value is a true claim: true asserts class membership, false asserts an implicit receiver these functions do not have. The result was that exported C functions rendered as though instance-bound while only TU-private ones rendered static, and a consumer had no way to recover linkage without reinterpreting an established predicate. The justification written into the doc comment does not survive contact with the finding, and is corrected rather than quietly dropped: if is_static really meant "no implicit receiver", EVERY free function would carry it, not only the translation-unit-private ones. The expander emits is_static only when the field is true, so leaving it at the default makes NO claim — which is the honest encoding for a function the predicate does not describe. Linkage stays on CppFunction for a consumer reading the harvest directly; representing it in the shared IR needs its own predicate, and that is a deliberate ontology change, not something to obtain by overloading an existing one. Measured: the rendered manifest is BYTE-IDENTICAL before and after, and the corpus explains why — OpenGGS contains zero file-scope static functions, so the flag was constant-false across all 181 and the overload never fired here. Latent, like the diagnostics gap: this is a correctness fix for the next C corpus, where leptonica and zlib are full of them, not a repair of a wrong result. Harvest unchanged at 55 TUs, 0 failed, 0 skipped, 181 functions, 763 triples, 527 on the signature plane. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- .../ruff_cpp_spo/examples/harvest_openggs.rs | 13 +++++++++++- crates/ruff_cpp_spo/src/lib.rs | 21 +++++++++++++------ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index c0b5a62f1c3f1..d7248ae3ac32e 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -145,11 +145,22 @@ fn main() -> Result<(), Box> { continue; } n_functions += 1; + // `CppFunction::is_static` (C internal linkage) is deliberately NOT + // mapped onto `CppMethod::is_static`. That predicate means "a + // class-level member with no implicit `this`", and a free function + // is not a class member at all, so neither value is a true claim + // about it: `true` would assert class membership, `false` would + // assert an implicit receiver these functions do not have. The + // expander emits `is_static` only when the field is true, so + // leaving it at the default makes NO claim — which is the honest + // encoding. Linkage stays on `CppFunction` for a consumer reading + // the harvest directly; representing it in the shared IR would need + // its own predicate, and that is a deliberate ontology change, not + // something to smuggle in by overloading an existing one. model.methods.push(CppMethod { name: f.name, return_type: f.return_type, param_types: f.param_types, - is_static: f.is_static, ..CppMethod::default() }); } diff --git a/crates/ruff_cpp_spo/src/lib.rs b/crates/ruff_cpp_spo/src/lib.rs index 3ecc7de0b61bc..1d814b17efd87 100644 --- a/crates/ruff_cpp_spo/src/lib.rs +++ b/crates/ruff_cpp_spo/src/lib.rs @@ -166,14 +166,23 @@ pub struct CppFunction { /// signature, so a downstream `MethodSig` manifest would carry an empty /// parameter list for every entry — a signature plane with no signatures. pub param_types: Vec, - /// `static` at file scope — internal linkage, i.e. NOT part of the + /// `static` at file scope — C internal linkage, i.e. NOT part of the /// library's API surface. /// - /// This is the C meaning of `static` (translation-unit-private), which is a - /// different fact from [`CppMethod::is_static`]'s class-level member. It - /// rides the same `is_static` predicate because both answer "no implicit - /// receiver, not an instance-bound call"; the distinction that matters to a - /// transcode — is this callable from outside the TU — is the one captured. + /// This is a DIFFERENT fact from `CppMethod::is_static`, which means "a + /// class-level member with no implicit `this`", and it must not be mapped + /// onto that predicate. The tempting justification — that both mean "no + /// implicit receiver" — does not survive contact: if it did, EVERY free + /// function would be `is_static`, not just the translation-unit-private + /// ones, and an exported C function would render as though it were an + /// instance method. Consumers would also have no way to recover linkage + /// without reinterpreting an established predicate. + /// + /// So linkage lives here, on the frontend type, and is deliberately left + /// out of the shared IR: the closed vocabulary has no predicate for it, and + /// adding one is a deliberate ontology change rather than something to + /// obtain by overloading. See `examples/harvest_openggs.rs` for the mapping + /// that omits it, and why omission is a truer encoding than either value. pub is_static: bool, } From b13168b4e6d17c14cb269ca793cae18735e5e365 Mon Sep 17 00:00:00 2001 From: Jan Date: Fri, 11 Sep 2026 04:04:02 +0000 Subject: [PATCH 4/7] Key the harvest dedup on the signature, and stop stamping another corpus's parity blocker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings on 5fb47858, both verified. Overload loss (Major). The per-model dedup keyed on f.name alone. This corpus is compiled as C++, so f(int) and f(double) are two distinct functions a name-only key collapses into one, dropping the second before it reaches the graph. The oracle is blind to it for the same reason it was blind to the diagnostics gap: it compares a projection of the graph against an expansion of the SAME graph, and both are equally short. The key is now (name, ordered parameter types), matching the identity the method IRI already uses. Measured: 181 functions before, 181 after, and a scan of the rendered manifest finds zero duplicate names within any translation unit. So OpenGGS has no overloaded free functions and the key never collided here — latent, like the previous two, and a correctness fix for the next C++-compiled corpus rather than a repair of a wrong count. Parity marker (Minor; codex, CodeRabbit and the PR description all flagged it). render hardcoded "PARITY: UNRUN (operator-blocked: leptonica)", so every OpenGGS artifact asserted an operator block from an unrelated corpus. An artifact stating something false about its own verification is worse than one that says nothing, because a reader who trusts the marker is misled about what was checked. Fixed additively so no existing caller changes behaviour: render_with_parity(manifests, parity) takes the status as a caller fact — which corpus is blocked, and on what, is known where the harvest runs, not in the renderer — and render delegates with the Tesseract arm's existing leptonica string. harvest_openggs now states its own: "UNRUN (no execution oracle: signatures only, bodies are hand-port)", which is the true status of this arm. Harvest unchanged: 55 TUs, 0 failed, 0 skipped, 181 functions, 763 triples, 527 on the signature plane, oracle green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- crates/ruff_cpp_codegen/src/lib.rs | 2 +- crates/ruff_cpp_codegen/src/render.rs | 26 +++++++++++++++++-- .../ruff_cpp_spo/examples/harvest_openggs.rs | 23 +++++++++++++--- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/crates/ruff_cpp_codegen/src/lib.rs b/crates/ruff_cpp_codegen/src/lib.rs index 510a4660bbb11..17cefddffd1bf 100644 --- a/crates/ruff_cpp_codegen/src/lib.rs +++ b/crates/ruff_cpp_codegen/src/lib.rs @@ -36,7 +36,7 @@ mod manifest; mod render; pub use manifest::{ClassManifest, MethodSig, decompile, is_signature_plane, project}; -pub use render::render; +pub use render::{render, render_with_parity}; #[cfg(test)] mod integration_tests { diff --git a/crates/ruff_cpp_codegen/src/render.rs b/crates/ruff_cpp_codegen/src/render.rs index f0b5c587fac8b..2a3fa8fb74170 100644 --- a/crates/ruff_cpp_codegen/src/render.rs +++ b/crates/ruff_cpp_codegen/src/render.rs @@ -45,14 +45,36 @@ const HAND_PORT: &[(&str, &str, &str)] = &[ ), ]; -/// Render the method manifests as Rust source. See the module docs. +/// The parity status of the original Tesseract arm, whose byte-parity probe is +/// blocked on a leptonica build environment. +const TESSERACT_PARITY: &str = "UNRUN (operator-blocked: leptonica)"; + +/// Render the method manifests as Rust source, with the Tesseract arm's parity +/// status. See the module docs. +/// +/// A second corpus must NOT use this: the emitted marker names leptonica, and a +/// harvest of some other corpus would then publish an operator block that has +/// nothing to do with it. Call [`render_with_parity`] instead. #[must_use] pub fn render(manifests: &[ClassManifest]) -> String { + render_with_parity(manifests, TESSERACT_PARITY) +} + +/// Render the method manifests as Rust source, stating `parity` as the emitted +/// `PARITY:` status. +/// +/// The status is a caller fact, not a renderer fact — which corpus is blocked, +/// and on what, is known where the harvest runs. Hardcoding one corpus's +/// blocker made every other consumer's artifact assert something false about +/// itself, which is worse than saying nothing: a reader who trusts the marker +/// is misled about what was verified. +#[must_use] +pub fn render_with_parity(manifests: &[ClassManifest], parity: &str) -> String { // Writing to a `String` via `fmt::Write` is infallible; the `Result` is // discarded deliberately. let mut out = String::new(); let _ = writeln!(out, "//! @generated by ruff_cpp_codegen -- DO NOT EDIT."); - let _ = writeln!(out, "//! PARITY: UNRUN (operator-blocked: leptonica)"); + let _ = writeln!(out, "//! PARITY: {parity}"); let _ = writeln!(out, "//!"); let _ = writeln!( out, diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index d7248ae3ac32e..8a02bc8477f48 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -136,12 +136,20 @@ fn main() -> Result<(), Box> { continue; } // A function DEFINED in a header reachable from several TUs is harvested - // once per TU. Keeping the first sighting per name inside one model is a - // corpus decision, not a dedup of occurrences. + // once per TU. Keeping the first sighting inside one model is a corpus + // decision, not a dedup of occurrences. + // + // The key is (name, ordered parameter types), NOT the name alone: this + // corpus is compiled as C++, so `f(int)` and `f(double)` are two + // distinct functions that a name-only key would collapse into one, + // silently dropping the second before it ever reached the graph. The + // oracle cannot catch that either — it compares a projection of the + // graph against an expansion of the SAME graph, and both are equally + // short. This key matches the identity the method IRI already uses. let mut seen = BTreeSet::new(); let mut model = Model::new(&stem); for f in funcs { - if !seen.insert(f.name.clone()) { + if !seen.insert((f.name.clone(), f.param_types.clone())) { continue; } n_functions += 1; @@ -205,7 +213,14 @@ fn main() -> Result<(), Box> { let out = PathBuf::from(env_or("GGS_OUT", ".claude/harvest/openggs")); std::fs::create_dir_all(&out)?; - let rendered = ruff_cpp_codegen::render(&manifests); + // This corpus's own parity status, not the Tesseract arm's. `render` would + // stamp "operator-blocked: leptonica" onto an artifact that has nothing to + // do with leptonica — an artifact asserting something false about its own + // verification is worse than one that says nothing. + let rendered = ruff_cpp_codegen::render_with_parity( + &manifests, + "UNRUN (no execution oracle: signatures only, bodies are hand-port)", + ); std::fs::write(out.join("openggs_methods.rs"), &rendered)?; std::fs::write(out.join("triples.ndjson"), to_ndjson(&triples))?; From 990a685f85be2d8e08319dbd57ac6b12e8ebf5ea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 05:08:42 +0000 Subject: [PATCH 5/7] Assert the calls invariant instead of describing it, and stop asserting an unchecked round-trip conformance Two findings from the review on b13168b4, both verified in-tree before fixing. The calls overload is the unguarded one. The is_static comment closes on the right principle -- a fact needing its own predicate must not be smuggled in by overloading an existing one -- and says nothing about `calls`, which is the other deliberately-absent field and the only one where that overload is a one-token edit. Verified, all four halves of the blindness claim: - CppFunction::calls is "EVERY CallExpr callee" as a bare name (ruff_cpp_spo/src/lib.rs:148-151); CppMethod::calls is the closed ActiveRecord mutator set as "." and means "this method calls a writer" (ruff_spo_triplet/src/ir.rs:321-328). Same name, same Vec, and the construction site uses ..CppMethod::default(). - The oracle cannot see it: `calls` is absent from is_signature_plane (ruff_cpp_codegen/src/manifest.rs:178-189). - A truth-leg check could not either: both Predicate::Calls emission sites stamp Provenance::Inferred (expand.rs:305 and :941), so f/c are identical whichever field feeds them. So the edit would mint every-callee facts as lifecycle-mutator claims with a green build, green clippy and a green oracle. The only thing standing in the way was a doc comment on the other struct, and AGENTS.md says a doc-comment claim is not a behaviour. It is now an assert on the built graph. One correction to the review's framing, checked rather than accepted: the Predicate::Calls site at expand.rs:305 is the AR-shape Function arm, not the CppFunction arm -- its reads are model-qualified and it carries traverses and guarded_writes. So the conclusion stands (the free-function call graph reaches no triple) but for the stated reason, not because no arm emits Calls at all. The round-trip conformance was an unchecked cross-repo claim. Three doc comments named codegen_spine::roundtrip_eq as the pattern this crate implements. That symbol appears nowhere in this repo outside those comments; this crate's only dependency is ruff_spo_triplet and its own description says it never compiles against lance-graph, so the function cannot be called and nothing fails if the two drift. One divergence is already real: roundtrip_eq's truth leg is documented as always run, while the comparison here keys on (s, p, o) and drops f/c -- sound on this plane only because expand and decompile assign identical provenance to all seven signature predicates, which is a property of the current expander rather than a guarantee. The three sites now say "mirrors" rather than "is", and the module doc records that the correspondence is asserted, not checked, and partial. Same class of fix as the PARITY marker in the previous commit: an artifact should not state something false about its own verification. Out of scope and deliberately not done: the missing dispatches_to predicate for the free-function call graph. That is an ontology change, which by this arm's own principle is a separate deliberate act. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- crates/ruff_cpp_codegen/src/lib.rs | 7 +++-- crates/ruff_cpp_codegen/src/manifest.rs | 18 ++++++++++-- .../ruff_cpp_spo/examples/harvest_openggs.rs | 28 +++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/ruff_cpp_codegen/src/lib.rs b/crates/ruff_cpp_codegen/src/lib.rs index 17cefddffd1bf..3c7a72b71ee57 100644 --- a/crates/ruff_cpp_codegen/src/lib.rs +++ b/crates/ruff_cpp_codegen/src/lib.rs @@ -26,9 +26,10 @@ //! //! [`decompile`] regenerates the signature-plane triples a manifest encodes; //! the round-trip `decompile(project(g))` must equal `expand(g)` restricted to -//! the signature plane. That is the `codegen_spine::roundtrip_eq` pattern over -//! the *live harvested* triples (see `manifest::tests`): a manifest that drops a -//! method or mangles a parameter fails it. A green render run is NOT byte-parity +//! the signature plane. That MIRRORS the `codegen_spine::roundtrip_eq` pattern +//! over the *live harvested* triples (see `manifest::tests`): a manifest that +//! drops a method or mangles a parameter fails it. "Mirrors", not "conforms to" +//! — see `manifest`'s module doc for why the correspondence is unchecked. A green render run is NOT byte-parity //! with libtesseract (that is the operator-gated `PROBE-OGAR-ADAPTER-UNICHARSET`); //! every emitted file says so via its `PARITY: UNRUN` marker. diff --git a/crates/ruff_cpp_codegen/src/manifest.rs b/crates/ruff_cpp_codegen/src/manifest.rs index d7a7571b68914..926441e6819a2 100644 --- a/crates/ruff_cpp_codegen/src/manifest.rs +++ b/crates/ruff_cpp_codegen/src/manifest.rs @@ -24,8 +24,19 @@ //! method, mangles a parameter, loses a return type, or misses an override edge //! produces a different `(s, p, o)` set and fails the round-trip. This is the //! `codegen_spine::roundtrip_eq` pattern (project → decompile → compare against -//! the live harvested triples), implemented over `ruff_spo_triplet::Triple` so +//! the live harvested triples), reimplemented over `ruff_spo_triplet::Triple` so //! the codegen crate stays `ruff_spo_triplet`-only (no lance-graph edge). +//! +//! **The correspondence is asserted, not checked, and is known to be partial.** +//! `roundtrip_eq` lives in `lance-graph-contract`, which this crate deliberately +//! does not depend on, so nothing here can call it and nothing fails if the two +//! drift. One divergence is already known: `roundtrip_eq`'s truth leg is +//! documented as always run (`0.0` does not skip it), while the comparison here +//! keys on `(s, p, o)` and drops `f`/`c`. That is sound for this plane only +//! because `expand` and `decompile` assign identical provenance to all seven +//! signature predicates — a property of the current expander, not a guarantee. +//! Read every `roundtrip_eq` mention in this crate as naming the pattern being +//! copied, never as a verified equivalence. use ruff_spo_triplet::{CppMethod, ModelGraph, Predicate, Provenance, Triple}; @@ -252,8 +263,9 @@ mod tests { } /// The teeth: regenerating the signature plane from the projected manifest - /// equals `expand`'s signature-plane output for the same graph. This is the - /// `codegen_spine::roundtrip_eq` pattern over the live harvested triples. + /// equals `expand`'s signature-plane output for the same graph. This mirrors + /// the `codegen_spine::roundtrip_eq` pattern over the live harvested triples + /// (unchecked correspondence — see the module doc). #[test] fn decompile_roundtrips_signature_plane_against_expand() { let g = fixture(); diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index 8a02bc8477f48..b2cc163d8d0a9 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -175,6 +175,34 @@ fn main() -> Result<(), Box> { graph.models.push(model); } + // `CppFunction::calls` is deliberately NOT mapped onto `CppMethod::calls`, + // and unlike the `is_static` decision above this one has NO guard behind it + // — so the invariant is asserted here rather than described. + // + // The two fields share a name and a `Vec` and mean different + // things: `CppFunction::calls` is EVERY `CallExpr` callee as a bare name, + // while `CppMethod::calls` is the closed `ActiveRecord` mutator set as + // `"."` and expands to "this method calls a writer". + // Mapping one onto the other would mint every-callee facts as + // lifecycle-mutator claims, and every guard in this arm is blind to it: + // the compiler and clippy see the same name and type; the oracle below + // cannot see it because `Calls` is not on the signature plane + // (`ruff_cpp_codegen::is_signature_plane`); and a truth-leg check could + // not either, because both `Predicate::Calls` emission sites stamp + // `Provenance::Inferred`, so `f`/`c` are identical either way. + // + // The free-function call graph wants its own predicate — the other + // `walk_free_functions` consumer already prints it as `dispatches_to` — + // and that is a deliberate ontology change, not this example's to make. + assert!( + graph + .models + .iter() + .flat_map(|m| &m.methods) + .all(|m| m.calls.is_empty()), + "CppFunction::calls (every callee) must not be mapped onto CppMethod::calls (the mutator set only)" + ); + let triples = expand(&graph); // ── THE ORACLE ─────────────────────────────────────────────────────────── From f9bd0beac6ac4f16376a8f3a54044cc23968dced Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 05:12:46 +0000 Subject: [PATCH 6/7] Fix two long lines this change introduced Both self-inflicted, and the first is a small lesson worth recording. The assert message was written as a backslash-continued string literal. cargo fmt collapsed it onto one line and kept the continuation's leading whitespace as literal spaces, so the committed message read "mapped onto " with ten embedded spaces across 116 columns. The point of that message is to tell a developer which of two same-named fields they just conflated, and a mangled message is a poor way to say it. It is now one short single-line string. The second is a sentence added to the lib.rs gate doc that ran onto the existing following line, making it 114 columns. Split into its own paragraph. The 202-column line remaining in harvest_openggs.rs is pre-existing (it was line 180 before this branch's previous commit) and is left alone rather than widening this change. Re-verified after the edits: ruff_cpp_spo 74 + 6 passing, ruff_cpp_codegen 10 passing, clippy clean on both with --all-targets, cargo fmt --check clean, and the harvest unchanged at 55 TUs / 0 failed / 0 skipped / 181 functions / 763 triples / 527 on the signature plane with the oracle green. The guard was disable-verified before this fix: adding calls: f.calls.clone() to the CppMethod literal -- the exact one-token overload it exists to catch -- panics the harvest, and removing it returns the run to green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- crates/ruff_cpp_codegen/src/lib.rs | 4 +++- crates/ruff_cpp_spo/examples/harvest_openggs.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ruff_cpp_codegen/src/lib.rs b/crates/ruff_cpp_codegen/src/lib.rs index 3c7a72b71ee57..802987c1e2e1a 100644 --- a/crates/ruff_cpp_codegen/src/lib.rs +++ b/crates/ruff_cpp_codegen/src/lib.rs @@ -29,7 +29,9 @@ //! the signature plane. That MIRRORS the `codegen_spine::roundtrip_eq` pattern //! over the *live harvested* triples (see `manifest::tests`): a manifest that //! drops a method or mangles a parameter fails it. "Mirrors", not "conforms to" -//! — see `manifest`'s module doc for why the correspondence is unchecked. A green render run is NOT byte-parity +//! — see `manifest`'s module doc for why the correspondence is unchecked. +//! +//! A green render run is NOT byte-parity //! with libtesseract (that is the operator-gated `PROBE-OGAR-ADAPTER-UNICHARSET`); //! every emitted file says so via its `PARITY: UNRUN` marker. diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index b2cc163d8d0a9..79da480db9ed1 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -200,7 +200,7 @@ fn main() -> Result<(), Box> { .iter() .flat_map(|m| &m.methods) .all(|m| m.calls.is_empty()), - "CppFunction::calls (every callee) must not be mapped onto CppMethod::calls (the mutator set only)" + "CppFunction::calls is every callee; CppMethod::calls is the mutator set only" ); let triples = expand(&graph); From e59b86bf568d892544b4727a10d727d510f3ff93 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 05:22:04 +0000 Subject: [PATCH 7/7] Key free-function identity on the qualified name, not the bare one CodeRabbit finding on f9bd0bea, verified before fixing. CppFunction::name is bare and the enclosing scope is captured separately in CppFunction::namespace, which the walker does populate (clang_walker.rs:324 via enclosing_scopes, and per its own doc that covers an owning class scope too, so Widget::helper yields ["Widget"]). The dedup key used only (name, param_types), so a::f(int) and b::f(int) collapsed to one entry and the second was dropped before it reached the graph. The finding's second half is the sharper one and also checks out: the method IRI is "{model_iri}.{name}({params})" with no scope component (expand.rs:784-792), so even had both survived the dedup they would have collided there. Fixing the key alone would have moved the silent loss rather than closed it. This mirrors an identity the class plane already established rather than inventing one: CppClass::qualified_name is used as the dedup key in walk_tu (lib.rs:364) and as the Model name in model_from_class (lib.rs:435). CppFunction::qualified_name is the free-function counterpart, with the same shape and the same global-scope-stays-bare rule, and it now feeds both the dedup key and the name that reaches the IR. Putting the qualified name in CppMethod::name is deliberate and is not the overload this arm keeps rejecting: a scope-qualified name is still a name, it is how C++ writes the identity, and the alternative -- a namespace field on the shared CppMethod -- is an ontology change. The renderer is unaffected because it emits the name as an escaped string literal, never as a Rust identifier. The regression test asserts both arms, since one alone passes for the wrong reason: a global-scope function must keep its bare name (the C-library norm, where qualifying would change every identity), and a namespaced pair must come out distinct. It also carries the anti-vacuity arm -- the fixture is asserted to still collide on the bare-name key being replaced, so the qualification is load-bearing rather than cosmetic. Measured: 75 + 6 passing (one new), clippy clean, fmt clean, and the harvest is BYTE-IDENTICAL before and after on both artifacts -- the rendered manifest and the NDJSON triples. OpenGGS is plain C with no namespaces, so all 181 functions have an empty scope and the qualified name equals the bare one. Latent here, like the diagnostics gap, the is_static overload and the overload dedup before it; this is a correctness fix for the next namespaced corpus, not a repair of a wrong count. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HScwwezRdMxFfTs3WLG19d --- .../ruff_cpp_spo/examples/harvest_openggs.rs | 32 +++++--- crates/ruff_cpp_spo/src/lib.rs | 75 +++++++++++++++++++ 2 files changed, 98 insertions(+), 9 deletions(-) diff --git a/crates/ruff_cpp_spo/examples/harvest_openggs.rs b/crates/ruff_cpp_spo/examples/harvest_openggs.rs index 79da480db9ed1..f20e15103d79b 100644 --- a/crates/ruff_cpp_spo/examples/harvest_openggs.rs +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -139,17 +139,26 @@ fn main() -> Result<(), Box> { // once per TU. Keeping the first sighting inside one model is a corpus // decision, not a dedup of occurrences. // - // The key is (name, ordered parameter types), NOT the name alone: this - // corpus is compiled as C++, so `f(int)` and `f(double)` are two - // distinct functions that a name-only key would collapse into one, - // silently dropping the second before it ever reached the graph. The - // oracle cannot catch that either — it compares a projection of the - // graph against an expansion of the SAME graph, and both are equally - // short. This key matches the identity the method IRI already uses. + // The key is (QUALIFIED name, ordered parameter types), and both halves + // are load-bearing. Parameter types, because this corpus is compiled as + // C++, so `f(int)` and `f(double)` are two distinct functions a + // name-only key would collapse into one. The qualified name, because + // `CppFunction::name` is bare while scope is captured separately, so + // `a::f(int)` and `b::f(int)` collide on the bare name — and they would + // then collide again in the method IRI, which is + // `{model_iri}.{name}({params})` with no scope component. + // + // The oracle cannot catch either collapse: it compares a projection of + // the graph against an expansion of the SAME graph, so both sides are + // equally short. The class plane already uses the qualified name for + // exactly this — as its dedup key in `walk_tu` and as the `Model::name` + // in `model_from_class` — so this mirrors an established identity + // rather than inventing one. let mut seen = BTreeSet::new(); let mut model = Model::new(&stem); for f in funcs { - if !seen.insert((f.name.clone(), f.param_types.clone())) { + let identity = f.qualified_name(); + if !seen.insert((identity.clone(), f.param_types.clone())) { continue; } n_functions += 1; @@ -165,8 +174,13 @@ fn main() -> Result<(), Box> { // the harvest directly; representing it in the shared IR would need // its own predicate, and that is a deliberate ontology change, not // something to smuggle in by overloading an existing one. + // The qualified name is what reaches the IR, so the method IRI + // carries the scope the shared `CppMethod` has no field for. + // Adding one would be an ontology change; a scope-qualified name is + // still a name, and it is how C++ itself writes the identity. At + // global scope this is the bare name, so a C corpus is unaffected. model.methods.push(CppMethod { - name: f.name, + name: identity, return_type: f.return_type, param_types: f.param_types, ..CppMethod::default() diff --git a/crates/ruff_cpp_spo/src/lib.rs b/crates/ruff_cpp_spo/src/lib.rs index 1d814b17efd87..09d1efec40f0e 100644 --- a/crates/ruff_cpp_spo/src/lib.rs +++ b/crates/ruff_cpp_spo/src/lib.rs @@ -186,6 +186,30 @@ pub struct CppFunction { pub is_static: bool, } +#[cfg(feature = "libclang")] +impl CppFunction { + /// The fully-qualified name (`a::f`) — the free-function counterpart of + /// [`CppClass::qualified_name`]. Joins [`Self::namespace`] components with + /// `::` and appends [`Self::name`]; returns the bare name at global scope, + /// which is the norm for a C library. + /// + /// This, not [`Self::name`], is the function's IDENTITY. `namespace` is + /// captured separately by the walker (and covers an owning class scope, so + /// `Widget::helper` yields `["Widget"]`), so the bare name alone does not + /// distinguish `a::f(int)` from `b::f(int)` — two different functions that + /// a name-keyed consumer silently collapses into one. The class plane + /// already treats the qualified name this way: it is both the dedup key + /// (`walk_tu`) and the [`Model::name`] (`model_from_class`). + #[must_use] + pub fn qualified_name(&self) -> String { + if self.namespace.is_empty() { + self.name.clone() + } else { + format!("{}::{}", self.namespace.join("::"), self.name) + } + } +} + /// A C or C++ enum declaration, with fully-explicit variant values. /// /// libclang exposes each `EnumConstantDecl` child with its resolved @@ -615,6 +639,57 @@ mod tests { assert!(triples.iter().all(|t| t.s.starts_with("cpp:"))); } + /// The free-function counterpart, and the reason it exists: a bare name + /// does not distinguish `a::f` from `b::f`, so a name-keyed dedup drops one + /// of them silently. Both arms are asserted — global scope must stay bare + /// (the C-library norm, where qualifying would change every identity) and a + /// namespaced pair must come out distinct. + #[test] + fn free_function_qualified_name_distinguishes_namespaces() { + let bare = CppFunction { + name: "pixScale".to_string(), + ..CppFunction::default() + }; + assert_eq!( + bare.qualified_name(), + "pixScale", + "a global-scope C function must keep its bare name" + ); + + let in_a = CppFunction { + namespace: vec!["a".to_string()], + name: "f".to_string(), + param_types: vec!["int".to_string()], + ..CppFunction::default() + }; + let in_b = CppFunction { + namespace: vec!["b".to_string()], + name: "f".to_string(), + param_types: vec!["int".to_string()], + ..CppFunction::default() + }; + assert_eq!(in_a.qualified_name(), "a::f"); + assert_ne!( + in_a.qualified_name(), + in_b.qualified_name(), + "a::f(int) and b::f(int) must not share an identity" + ); + // The anti-vacuity arm: the two are indistinguishable by the key this + // replaces, so the qualification is load-bearing rather than cosmetic. + assert_eq!( + (&in_a.name, &in_a.param_types), + (&in_b.name, &in_b.param_types), + "the fixture no longer collides on the bare-name key it guards" + ); + + let nested = CppFunction { + namespace: vec!["tesseract".to_string(), "lstm".to_string()], + name: "helper".to_string(), + ..CppFunction::default() + }; + assert_eq!(nested.qualified_name(), "tesseract::lstm::helper"); + } + #[test] fn qualified_name_joins_namespace() { let cls = CppClass {