diff --git a/crates/ruff_cpp_spo/src/clang_walker.rs b/crates/ruff_cpp_spo/src/clang_walker.rs index 040d58634686e..bc54c8d31e21a 100644 --- a/crates/ruff_cpp_spo/src/clang_walker.rs +++ b/crates/ruff_cpp_spo/src/clang_walker.rs @@ -42,14 +42,14 @@ use std::path::Path; use clang::diagnostic::Severity; use clang::{ Accessibility, Clang, Entity, EntityKind, ExceptionSpecification, Index, RefQualifier, - StorageClass, + StorageClass, TypeKind, }; use ruff_spo_triplet::{ CppAccess, CppBase, CppField, CppFriend, CppMethod, CppRefQualifier, CppTemplate, CppTemplateKind, }; -use crate::{CppClass, CppEnum, CppFunction, Declaration}; +use crate::{CppClass, CppEnum, CppFunction, Declaration, LookupSet}; /// A failure walking a translation unit. #[derive(Debug)] @@ -297,10 +297,17 @@ fn collect_functions(entity: &Entity, out: &mut Vec) { } else { name }; - let mut calls = Vec::new(); - collect_calls(&child, &mut calls); + let mut found = FoundCalls::default(); + collect_calls(&child, &mut found); + let FoundCalls { + mut calls, + mut unresolved_calls, + dynamic_calls, + } = found; calls.sort(); calls.dedup(); + unresolved_calls.sort(); + unresolved_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 @@ -328,6 +335,8 @@ fn collect_functions(entity: &Entity, out: &mut Vec) { namespace: enclosing_scopes(&child), name, calls, + unresolved_calls, + dynamic_calls, return_type, param_types, is_static, @@ -341,60 +350,166 @@ fn collect_functions(entity: &Entity, out: &mut Vec) { } } -/// Recurse a function body collecting EVERY resolvable callee name (the general -/// call graph). Distinct from [`walk_body`]'s `calls` (persistence mutators -/// only): here every `CallExpr` callee is the dispatch structure a C-library -/// transcode follows. +/// The calls of one function body, by how far libclang resolved them. #[cfg(feature = "libclang")] -fn collect_calls(node: &Entity, out: &mut Vec) { +#[derive(Default)] +struct FoundCalls { + calls: Vec, + unresolved_calls: Vec, + dynamic_calls: u32, +} + +/// Where a `CallExpr` goes, as far as libclang resolved it. +#[cfg(feature = "libclang")] +enum CallTarget { + /// A function or method declaration: its scope-qualified name. + Resolved(String), + /// An unresolved lookup set (`OverloadedDeclRef`). + Lookup(LookupSet), + /// Anything else: a pointer, a parameter, a field. Not named. + Dynamic, +} + +/// Recurse a function body collecting EVERY call (the general call graph). +/// Distinct from [`walk_body`]'s `calls` (persistence mutators only): here +/// every `CallExpr` is the dispatch structure a C-library transcode follows, +/// split by resolution status so an unresolved call is never reported as a +/// resolved one. +#[cfg(feature = "libclang")] +fn collect_calls(node: &Entity, out: &mut FoundCalls) { for child in node.get_children() { - if child.get_kind() == EntityKind::CallExpr - && let Some(name) = call_callee_name(&child) - { - out.push(name); + if child.get_kind() == EntityKind::CallExpr { + match call_target(&child) { + CallTarget::Resolved(name) => out.calls.push(name), + CallTarget::Lookup(set) => out.unresolved_calls.push(set), + CallTarget::Dynamic => out.dynamic_calls += 1, + } } collect_calls(&child, out); } } -/// The callee name of a `CallExpr` cursor, with a fallback for the case -/// `child.get_name()` alone misses. +/// The target of a `CallExpr` cursor. +/// +/// A resolved call references the callee's declaration. A METHOD is emitted +/// class-qualified (`A::reset`) so it joins against the class-qualified +/// definition and same-named methods of different classes never collapse +/// (codex P2 on ruff #57). A free function carries its enclosing scopes +/// (`a::f`), the key [`CppFunction::qualified_name`] gives its definition, so +/// `a::f` and `b::f` never collapse; at global scope (the C norm) it is bare. +/// +/// When anything in the surrounding expression has an error or dependent +/// type, Clang represents the callee as an unresolved lookup set +/// (`OverloadedDeclRef`), even for an ordinary, non-overloaded function. The +/// concrete case is Tesseract's makerow.cpp `make_baseline_spline` +/// (`tesseract-rs/.claude/harvest/makerow-callgraph.txt`): an `auto` variable +/// whose initializer names an undeclared symbol becomes ``, +/// and passing it as an argument turns the callee into a lookup set. Overload +/// resolution did not happen, so that call is a [`CallTarget::Lookup`] with +/// its candidates, never a resolved call. The set is searched among the +/// `CallExpr`'s descendants, stopping at any nested `CallExpr` (an argument +/// that is itself a call) so a broken INNER call's set is never mistaken for +/// the OUTER one. /// -/// Ordinarily `CallExpr::get_name()` (`clang_getCursorSpelling`) already -/// resolves the callee — but when ANYTHING in the surrounding expression has -/// an error/dependent type (a genuinely unresolved template-dependent call, -/// OR — the concrete case found harvesting real Tesseract, makerow.cpp's -/// `make_baseline_spline`, `tesseract-rs/.claude/harvest/makerow-callgraph.txt` -/// — a call downstream of an UNRELATED parse error elsewhere in the same -/// statement: an `auto`-typed variable whose initializer references an -/// undeclared symbol becomes ``, and passing THAT variable as -/// an argument to an otherwise perfectly ordinary, non-overloaded function -/// forces Clang to represent the call's callee as an `UnresolvedLookupExpr` -/// instead of a resolved `DeclRefExpr`), `clang_getCursorSpelling` on the -/// `CallExpr` itself returns empty (`get_name()` → `None`) even though the -/// callee's own name is perfectly well-formed one level down: nested inside -/// an `OverloadedDeclRef` cursor (libclang's cursor-kind for the unresolved -/// lookup set), reachable via the `CallExpr`'s callee sub-expression -/// (`DeclRefExpr`/`UnexposedExpr` wrapping). Falls back to the first -/// `OverloadedDeclRef` found among the `CallExpr`'s descendants, stopping at -/// any nested `CallExpr` (an argument that is itself a call) so a broken -/// INNER call's callee is never mistaken for the OUTER one. +/// A call that references anything else (a function pointer variable, a +/// parameter, a field) is [`CallTarget::Dynamic`]: the program text does not +/// name the target, and the variable's name is not a callee. +/// +/// A constructor or destructor call names its target statically and is +/// resolved like a method (`A::A`). A call whose callee expression is itself +/// a call returning a function pointer (`factory()(1)`) is +/// [`CallTarget::Dynamic`], even though libclang reports the inner function +/// as the outer call's reference; the inner call is a separate `CallExpr` +/// and keeps its own resolved edge. #[cfg(feature = "libclang")] -fn call_callee_name(call: &Entity) -> Option { - // A resolved call to a METHOD is emitted class-qualified (`A::reset`) so - // it joins against the class-qualified definition entry and same-named - // methods of different classes never collapse (codex P2 on ruff #57). - // Free-function callees stay bare (zero-loss vs the banked manifests); - // the OverloadedDeclRef fallback stays bare too — an unresolved lookup - // set has no single owning class to name. +fn call_target(call: &Entity) -> CallTarget { if let Some(referenced) = call.get_reference() { - if referenced.get_kind() == EntityKind::Method - && let Some(name) = referenced.get_name() - { - return Some(qualify_with_class(&referenced, name)); + if calls_a_returned_function(call, &referenced) { + return CallTarget::Dynamic; + } + return match referenced.get_kind() { + EntityKind::Method | EntityKind::Constructor | EntityKind::Destructor => { + referenced.get_name().map_or(CallTarget::Dynamic, |name| { + CallTarget::Resolved(qualify_with_class(&referenced, name)) + }) + } + EntityKind::FunctionDecl + | EntityKind::FunctionTemplate + | EntityKind::ConversionFunction => CallTarget::Resolved(qualified_name(&referenced)), + EntityKind::OverloadedDeclRef => CallTarget::Lookup(lookup_set(&referenced)), + _ => CallTarget::Dynamic, + }; + } + find_overloaded_decl_ref(call) + .map_or(CallTarget::Dynamic, |o| CallTarget::Lookup(lookup_set(&o))) +} + +/// Is `call`'s callee expression the result of another call to `referenced`, +/// a function returning a function pointer or reference? Then `call` invokes +/// whatever that function returned, not `referenced` itself. +/// +/// Both conditions are needed. The callee is the `CallExpr`'s first child, +/// but an operator call (`g() + x`) or a constructor (`A a(make())`) can also +/// have a call as its first child; neither references a function returning a +/// function pointer. +#[cfg(feature = "libclang")] +fn calls_a_returned_function(call: &Entity, referenced: &Entity) -> bool { + let returns_function = referenced.get_result_type().is_some_and(|t| { + let t = t.get_canonical_type(); + let pointee = match t.get_kind() { + TypeKind::Pointer | TypeKind::LValueReference | TypeKind::RValueReference => { + t.get_pointee_type() + } + TypeKind::MemberPointer => return true, + _ => None, + }; + pointee.is_some_and(|p| { + matches!( + p.get_canonical_type().get_kind(), + TypeKind::FunctionPrototype | TypeKind::FunctionNoPrototype + ) + }) + }); + if !returns_function { + return false; + } + let mut callee = call.get_children().into_iter().next(); + while let Some(node) = callee { + match node.get_kind() { + EntityKind::CallExpr => return true, + EntityKind::UnexposedExpr | EntityKind::ParenExpr => { + callee = node.get_children().into_iter().next(); + } + _ => return false, } } - call.get_name().or_else(|| find_overloaded_decl_ref(call)) + false +} + +/// The lookup set of an `OverloadedDeclRef` cursor: its spelled name and its +/// candidates, each scope-qualified with its signature. +#[cfg(feature = "libclang")] +fn lookup_set(set: &Entity) -> LookupSet { + let mut candidates: Vec = set + .get_overloaded_declarations() + .unwrap_or_default() + .iter() + .map(|d| { + let mut parts = enclosing_scopes(d); + parts.push( + d.get_display_name() + .or_else(|| d.get_name()) + .unwrap_or_default(), + ); + parts.join("::") + }) + .collect(); + candidates.sort(); + candidates.dedup(); + LookupSet { + name: set.get_name().unwrap_or_default(), + candidates, + } } /// `Class::name` for a method entity, from its SEMANTIC parent (the class), @@ -408,18 +523,18 @@ fn qualify_with_class(method: &Entity, name: String) -> String { } } +/// The first `OverloadedDeclRef` among `node`'s descendants, not entering a +/// nested `CallExpr`. #[cfg(feature = "libclang")] -fn find_overloaded_decl_ref(node: &Entity) -> Option { +fn find_overloaded_decl_ref<'tu>(node: &Entity<'tu>) -> Option> { for child in node.get_children() { - if child.get_kind() == EntityKind::OverloadedDeclRef - && let Some(name) = child.get_name() - { - return Some(name); + if child.get_kind() == EntityKind::OverloadedDeclRef { + return Some(child); } if child.get_kind() != EntityKind::CallExpr - && let Some(name) = find_overloaded_decl_ref(&child) + && let Some(found) = find_overloaded_decl_ref(&child) { - return Some(name); + return Some(found); } } None @@ -2400,14 +2515,22 @@ void Widget::dispatch() { dispatch.calls ); - // Fix 2: the unresolved (`OverloadedDeclRef`-shaped) callee reference - // to `free_helper` is still recovered, despite `get_name()` on the - // `CallExpr` itself returning empty. + // Fix 2, re-pinned (S2): the unresolved (`OverloadedDeclRef`-shaped) + // callee reference to `free_helper` is still recovered, but as an + // unresolved lookup set with its candidate, never as a resolved call + // (overload resolution did not happen). assert!( - compute.calls.contains(&"free_helper".to_string()), - "compute must call free_helper despite the unresolved-lookup shape: {:?}", + !compute.calls.contains(&"free_helper".to_string()), + "an unresolved lookup set must not be a resolved call: {:?}", compute.calls ); + assert_eq!( + compute.unresolved_calls, + [LookupSet { + name: "free_helper".to_string(), + candidates: vec!["free_helper(int)".to_string()], + }] + ); // Regression: an IN-CLASS (inline) method definition must NOT be // captured — this walker never recurses into a ClassDecl body, and @@ -2466,6 +2589,111 @@ void drive(A &a, B &b) { ); } + /// Identity gates (S2, N2/N3): a resolved callee keeps its namespace, so + /// `a::f` and `b::f` never collapse; a call libclang leaves as an + /// unresolved lookup set (`g` below, overloaded, called with a dependent + /// argument) is never reported as a resolved call; a call through a + /// function pointer is never reported as a call to the pointer's name. + const CALL_IDENTITY_SRC: &str = r" +namespace a { int f(int x) { return x; } } +namespace b { int f(int x) { return x + 1; } } +int g(int); +int g(double); +int use_ns(int x) { return a::f(x) + b::f(x); } +int use_broken(int x) { auto v = undeclared_symbol; return g(v); } +int via_ptr(int (*fp)(int), int x) { return fp(x); } +"; + + #[test] + fn call_identity_keeps_namespaces_and_never_resolves_a_lookup_set() { + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("call_identity", CALL_IDENTITY_SRC); + let funcs = walk_free_functions(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + let get = |n: &str| { + funcs + .iter() + .find(|f| f.name == n) + .unwrap_or_else(|| panic!("{n} missing; got {funcs:?}")) + }; + assert_eq!(get("use_ns").calls, ["a::f", "b::f"], "namespaces kept"); + assert!( + !get("use_broken").calls.contains(&"g".to_string()), + "an unresolved lookup set is not a resolved call: {:?}", + get("use_broken").calls + ); + assert!( + !get("via_ptr").calls.contains(&"fp".to_string()), + "a call through a pointer is not a call to the pointer: {:?}", + get("via_ptr").calls + ); + // Where the unresolved calls go instead: the lookup set keeps both + // overloads as distinct candidates; the pointer call is counted only. + assert_eq!( + get("use_broken").unresolved_calls, + [LookupSet { + name: "g".to_string(), + candidates: vec!["g(double)".to_string(), "g(int)".to_string()], + }] + ); + assert_eq!(get("via_ptr").dynamic_calls, 1); + assert!(get("via_ptr").unresolved_calls.is_empty()); + // The silence twin: resolved calls carry no unresolved or dynamic + // count. + assert!(get("use_ns").unresolved_calls.is_empty()); + assert_eq!(get("use_ns").dynamic_calls, 0); + } + + /// A constructor names its target statically, so it stays a resolved + /// call; a call through a function pointer RETURNED by another call is + /// dynamic, while the inner call keeps its resolved edge. Twins: a + /// constructor whose first argument is a call, and an overloaded operator + /// whose left operand is a call, both stay resolved with no dynamic count. + const CALL_SHAPES_SRC: &str = r" +struct A { A(int); }; +int make(); +struct N { int v; }; +N operator+(N, int); +N mk(); +int (*factory())(int); +void construct() { A a(1); } +void construct_from_call() { A a(make()); } +int through_returned() { return factory()(1); } +N add_call_lhs(int x) { return mk() + x; } +"; + + #[test] + fn constructors_resolve_and_returned_function_pointers_are_dynamic() { + let _guard = CLANG_TEST_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = write_fixture("call_shapes", CALL_SHAPES_SRC); + let funcs = walk_free_functions(&path, &cxx_args()).expect("libclang walk"); + let _ = std::fs::remove_file(&path); + let get = |n: &str| { + funcs + .iter() + .find(|f| f.name == n) + .unwrap_or_else(|| panic!("{n} missing; got {funcs:?}")) + }; + assert_eq!(get("construct").calls, ["A::A"]); + assert_eq!(get("construct").dynamic_calls, 0); + assert_eq!(get("through_returned").calls, ["factory"]); + assert_eq!(get("through_returned").dynamic_calls, 1); + // Twins. + assert_eq!(get("construct_from_call").calls, ["A::A", "make"]); + assert_eq!(get("construct_from_call").dynamic_calls, 0); + assert_eq!( + get("add_call_lhs").dynamic_calls, + 0, + "{:?}", + get("add_call_lhs") + ); + assert!(get("add_call_lhs").calls.contains(&"mk".to_string())); + } + /// A PLAIN free function (no class involved) keeps its existing /// zero-fallback behavior: `call_callee_name` only engages when /// `get_name()` is empty, so a healthy, already-resolved call is diff --git a/crates/ruff_cpp_spo/src/lib.rs b/crates/ruff_cpp_spo/src/lib.rs index 38a4b4823f0ad..b99dc69f956d5 100644 --- a/crates/ruff_cpp_spo/src/lib.rs +++ b/crates/ruff_cpp_spo/src/lib.rs @@ -145,10 +145,21 @@ pub struct CppFunction { pub namespace: Vec, /// The function name as written (`pixScaleGrayLI`). pub name: String, - /// Every resolvable callee name in the body, deduped + sorted — the call - /// graph. Distinct from the AR/OO `BodyArm.calls` (persistence mutators - /// only); this is EVERY `CallExpr` callee. + /// Every RESOLVED callee in the body, deduped + sorted — the call graph. + /// Distinct from the AR/OO `BodyArm.calls` (persistence mutators only). + /// A method is class-qualified (`A::reset`); a free function carries its + /// enclosing scopes (`a::f`, the same key as [`Self::qualified_name`]; + /// bare at global scope). Calls libclang did not resolve are NOT here: + /// see [`Self::unresolved_calls`] and [`Self::dynamic_calls`]. pub calls: Vec, + /// Calls libclang left as an unresolved lookup set (`OverloadedDeclRef`), + /// deduped + sorted. Never a resolved call: overload resolution did not + /// happen, so which candidate is called is unknown. + pub unresolved_calls: Vec, + /// Calls through something that is not a function declaration (a + /// function pointer, a parameter, a field): the target is not named by + /// the program text, so only the count is kept. + pub dynamic_calls: u32, /// 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". @@ -194,6 +205,18 @@ pub struct CppFunction { pub file: Option, } +/// An unresolved call: the name as spelled at the call site and the +/// declarations the lookup found, each scope-qualified with its signature +/// (`g(int)`, `g(double)`), so two overloads stay distinct. +#[cfg(feature = "libclang")] +#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord)] +pub struct LookupSet { + /// The name as spelled at the call site. + pub name: String, + /// The candidates of the lookup set, sorted and deduped. + pub candidates: Vec, +} + #[cfg(feature = "libclang")] impl CppFunction { /// The fully-qualified name (`a::f`) — the free-function counterpart of