diff --git a/crates/ruff_cpp_codegen/src/lib.rs b/crates/ruff_cpp_codegen/src/lib.rs index 510a4660bbb118..802987c1e2e1af 100644 --- a/crates/ruff_cpp_codegen/src/lib.rs +++ b/crates/ruff_cpp_codegen/src/lib.rs @@ -26,9 +26,12 @@ //! //! [`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. @@ -36,7 +39,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/manifest.rs b/crates/ruff_cpp_codegen/src/manifest.rs index d7a7571b689141..926441e6819a25 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_codegen/src/render.rs b/crates/ruff_cpp_codegen/src/render.rs index f0b5c587fac8b4..2a3fa8fb74170f 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/Cargo.toml b/crates/ruff_cpp_spo/Cargo.toml index 8b4c4d08f52e02..63ef98084bc58f 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 00000000000000..f20e15103d79bf --- /dev/null +++ b/crates/ruff_cpp_spo/examples/harvest_openggs.rs @@ -0,0 +1,291 @@ +//! 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_with_diagnostics}; +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) +} + +/// 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() { + 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, 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, 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; + } + // A function DEFINED in a header reachable from several TUs is harvested + // once per TU. Keeping the first sighting inside one model is a corpus + // decision, not a dedup of occurrences. + // + // 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 { + let identity = f.qualified_name(); + if !seen.insert((identity.clone(), f.param_types.clone())) { + 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. + // 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: identity, + return_type: f.return_type, + param_types: f.param_types, + ..CppMethod::default() + }); + } + 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 is every callee; CppMethod::calls is the mutator set only" + ); + + 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)?; + // 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))?; + + eprintln!( + "[ggs] {} TUs ({n_failed} failed to parse, {n_diagnostics} skipped on error \ + diagnostics), {} 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 93619bad9ca254..91190cf4e4ace3 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, @@ -197,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 @@ -206,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 @@ -260,10 +301,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 +2482,136 @@ 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 + /// 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 62e205f1b2f76b..09d1efec40f0e9 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. @@ -148,6 +149,65 @@ 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 — C internal linkage, i.e. NOT part of the + /// library's API surface. + /// + /// 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, +} + +#[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. @@ -579,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 {