diff --git a/.gitignore b/.gitignore index a9ebb65..edf239d 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,8 @@ target/ # built on demand rather than committed. The small packs under `infact-packs/` # are the ones this repository's own tests and documentation depend on. /generated-packs + +# The standard library's catalog. Generated the same way, but 26 MB rather than +# the few hundred kilobytes a library takes, and reproducible in two seconds +# from a rustup component. `infact-packs/rust-std/README.md` is the command. +/infact-packs/rust-std/api/*.json diff --git a/crates/infact-catalog/src/rustdoc.rs b/crates/infact-catalog/src/rustdoc.rs index 269d1ab..c76409f 100644 --- a/crates/infact-catalog/src/rustdoc.rs +++ b/crates/infact-catalog/src/rustdoc.rs @@ -84,6 +84,67 @@ pub fn build_catalog(request: CatalogRequest<'_>) -> Result { } } + // Methods written on a type rather than in a trait. They are associated + // items, so the free-function pass below skips them, and they belong to no + // trait, so the pass above never saw them: `Vec::push` and `<[T]>::sorted` + // fell between the two and the catalog held only what a library declared in + // traits. For core that was five thousand methods, and for the standard + // library it is most of what anyone calls. + for item in index.values() { + let item = object(item, "item")?; + let Some(implementation) = item + .get("inner") + .and_then(Value::as_object) + .and_then(|inner| inner.get("impl")) + .and_then(Value::as_object) + else { + continue; + }; + // A trait implementation's methods belong to the trait, which the pass + // above already catalogued from its declaration. + if implementation + .get("trait") + .is_some_and(|value| !value.is_null()) + { + continue; + } + let Some(container) = implemented_type(implementation.get("for"), paths) else { + continue; + }; + for method_id in implementation + .get("items") + .and_then(Value::as_array) + .into_iter() + .flatten() + { + let Some(method) = method_id + .as_u64() + .and_then(|id| index.get(&id.to_string())) + .and_then(Value::as_object) + else { + continue; + }; + let Some(function) = method + .get("inner") + .and_then(Value::as_object) + .and_then(|inner| inner.get("function")) + .and_then(Value::as_object) + else { + continue; + }; + let Some(name) = string(method.get("name")) else { + continue; + }; + callables.push(ExternalCallable { + path: format!("{container}::{name}"), + container: CallableContainer::Type { + path: container.clone(), + }, + signature: Some(signature(function)?), + }); + } + } + for (id, item) in index { if associated_items.contains(id) { continue; @@ -318,6 +379,33 @@ fn type_arguments(value: Option<&Value>) -> Result> { .collect() } +/// The name of the type an inherent implementation is written on. +/// +/// A nominal type is named by the path the crate publishes it under. A built-in +/// one has no path to look up and is named the way the language names it, which +/// is also how its documentation is addressed: `slice`, `str`, `u32`. Anything +/// else — a raw pointer, a function type — has no name a caller would write a +/// method path with, and is left out rather than given an invented one. +fn implemented_type(value: Option<&Value>, paths: &Map) -> Option { + let target = value?.as_object()?; + if let Some(resolved) = target.get("resolved_path").and_then(Value::as_object) { + let id = resolved.get("id")?.as_u64()?.to_string(); + // The published path is what a caller writes. Falling back to the bare + // name would put two types with one name under a single heading. + return public_path(paths, &id); + } + if let Some(primitive) = target.get("primitive").and_then(Value::as_str) { + return Some(primitive.to_owned()); + } + if target.contains_key("slice") { + return Some("slice".to_owned()); + } + if target.contains_key("array") { + return Some("array".to_owned()); + } + None +} + fn public_path(paths: &Map, id: &str) -> Option { paths .get(id)? diff --git a/crates/infact-core/src/lib.rs b/crates/infact-core/src/lib.rs index 24e54ce..f7ddd86 100644 --- a/crates/infact-core/src/lib.rs +++ b/crates/infact-core/src/lib.rs @@ -303,8 +303,21 @@ pub struct ExternalCallable { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum CallableContainer { - Trait { path: String }, - Module { path: String }, + Trait { + path: String, + }, + Module { + path: String, + }, + /// The type a method is written on, outside any trait. + /// + /// `Vec::push` and `<[T]>::is_sorted` belong to no trait and no module, and + /// without this they belonged to nothing and were dropped. In core alone + /// that is 5,621 methods — most of what a caller of the standard library + /// actually calls. + Type { + path: String, + }, } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] @@ -439,6 +452,14 @@ pub enum Condition { /// A quadratic scan of four elements beats allocating a hash set. Which one /// this is depends on the caller, and nothing in the callee says. SmallInputsFavourTheCode, + /// The API and the code disagree where two elements are incomparable. + /// + /// A hand-written sortedness check refuses only on a strict `a > b`; the + /// API accepts only on `a <= b`. Under a total order those are the same + /// question. Under a partial one they are not — two `f64` NaNs are neither + /// greater nor less than each other, so the loop calls the sequence sorted + /// and the API does not. + IncomparableElements, } impl std::fmt::Display for Condition { @@ -458,6 +479,9 @@ impl std::fmt::Display for Condition { Self::SmallInputsFavourTheCode => { formatter.write_str("the code is faster at small sizes") } + Self::IncomparableElements => formatter.write_str( + "two elements that cannot be compared are sorted to the code and not to the API", + ), } } } diff --git a/crates/infact-normalize/src/lib.rs b/crates/infact-normalize/src/lib.rs index e34cae1..079dc3e 100644 --- a/crates/infact-normalize/src/lib.rs +++ b/crates/infact-normalize/src/lib.rs @@ -364,6 +364,25 @@ pub enum Coverage { /// [`Coverage::Once`] about anything order- or count-sensitive and exactly /// as much about anything that is neither. BothWays, + /// Each element with the one after it: the `windows(2)` walk. + /// + /// A single loop, not a nested one, and the only coverage where `left` and + /// `right` are neighbours rather than an arbitrary pair. That makes it the + /// one that says something about ORDER: a test over adjacent pairs decides + /// whether the sequence is sorted, and the same test over every pair + /// decides something much stronger. + Adjacent, +} + +/// Whether a position is one past a named one. +/// +/// Canonicalized arithmetic puts the name first, but both orders are accepted +/// so that a rewrite does not depend on which way the ordering happened to fall. +fn is_next_position(form: &Form, index: u32) -> bool { + matches!(form, Form::Binary { operator, left, right } + if operator == "+" + && ((**left == Form::Local(index) && **right == Form::Number("1".to_owned())) + || (**right == Form::Local(index) && **left == Form::Number("1".to_owned())))) } /// Which way a walk runs. @@ -1127,6 +1146,68 @@ impl Form { Some(()) } + /// The largest binding number anything here introduces or mentions. + /// + /// A rewrite that turns one name into two needs a number for the second, + /// and reusing one already in play would silently identify two different + /// values. Renaming makes the numbers canonical afterwards, so any unused + /// one will do. + fn highest_binding(&self) -> Option { + let here = match self { + Self::Local(index) => Some(*index), + _ => None, + }; + self.children() + .into_iter() + .filter_map(Self::highest_binding) + .chain(here) + .max() + } + + /// Whether a body reads a sequence only by indexing it at a position and + /// its successor. + /// + /// The adjacent-pairs licence, and narrower than it looks: `v[i]` and + /// `v[i + 1]` are the only two readings allowed, so `v[i + 2]`, a bare `i`, + /// or any other use of `v` declines. Without that, a loop that happens to + /// read a neighbour among other things would be reported as a walk over + /// neighbours, which is not what it does. + fn adjacent_only(&self, sequence: &Self, index: u32) -> bool { + if let Self::Index { + sequence: indexed, + position, + } = self + && indexed.as_ref() == sequence + && (**position == Self::Local(index) || is_next_position(position, index)) + { + return true; + } + if self == sequence || *self == Self::Local(index) { + return false; + } + self.children() + .into_iter() + .all(|child| child.adjacent_only(sequence, index)) + } + + /// The same body with each neighbour reading replaced by its element. + fn with_adjacent_elements(&self, sequence: &Self, index: u32, successor: u32) -> Self { + if let Self::Index { + sequence: indexed, + position, + } = self + && indexed.as_ref() == sequence + { + if **position == Self::Local(index) { + return Self::Local(index); + } + if is_next_position(position, index) { + return Self::Local(successor); + } + } + self.map_children(&|child| child.with_adjacent_elements(sequence, index, successor)) + } + /// Whether a body reads a sequence only by indexing it at named positions. /// /// This is the licence to forget the index. `for i in 0..v.len()` visits @@ -1289,6 +1370,7 @@ impl Display for Form { let kind = match coverage { Coverage::Once => "pairwise", Coverage::BothWays => "pairwise-both-ways", + Coverage::Adjacent => "pairwise-adjacent", }; write!(formatter, "({kind} {sequence} {left} {right} {body})") } diff --git a/crates/infact-normalize/src/simplify.rs b/crates/infact-normalize/src/simplify.rs index 37d6709..3322c06 100644 --- a/crates/infact-normalize/src/simplify.rs +++ b/crates/infact-normalize/src/simplify.rs @@ -175,6 +175,7 @@ impl Form { .or_else(|| rebuilt.as_canonical_arithmetic()) .or_else(|| rebuilt.as_element_traversal()) .or_else(|| rebuilt.as_pairwise()) + .or_else(|| rebuilt.as_adjacent_pairwise()) .or_else(|| rebuilt.as_recovered_escape()) .or_else(|| rebuilt.as_unfolded(fuel)) .unwrap_or(rebuilt) @@ -479,6 +480,51 @@ impl Form { .or_else(|| Self::as_enumerated_pairwise(outer, first, inner, second, inner_body)) } + /// A single loop reading each element and the one after it. + /// + /// `for i in 0..v.len() - 1 { .. v[i] .. v[i + 1] .. }` is the `windows(2)` + /// walk written out, and unlike the other two coverages it is one loop + /// rather than two. The bound has to stop one short, because that is what + /// keeps `v[i + 1]` inside the sequence; a loop that ran to the end would + /// be a different walk, and in Rust a panicking one. + /// + /// The second element needs a name the body is not already using, since the + /// code has only one index where the form has two elements. + fn as_adjacent_pairwise(&self) -> Option { + let Self::Traverse { + sequence, + item, + body, + direction: Direction::Forward, + } = self + else { + return None; + }; + let Pattern::Binding(index) = item.as_ref() else { + return None; + }; + let (start, end) = counting_span(sequence)?; + if *start != Self::Number("0".to_owned()) { + return None; + } + // `0..n - 1` reads positions up to `n`, because the last step reads its + // neighbour. That is the extent, and it is why this cannot reuse the + // bound as written. + let extent = one_more_than(end)?; + let source = body.sole_indexed_sequence(&[*index])?; + if !body.adjacent_only(source, *index) || body.writes_indexed(source) { + return None; + } + let successor = self.highest_binding().map_or(0, |highest| highest + 1); + Some(Self::Pairwise { + sequence: Box::new(walked_sequence(start, &extent, source)), + left: item.clone(), + right: Box::new(Pattern::Binding(successor)), + body: Box::new(body.with_adjacent_elements(source, *index, successor)), + coverage: Coverage::Adjacent, + }) + } + /// The square spelling: two loops over the whole range, minus the diagonal. /// /// `for i in 0..n { for j in 0..n { if i != j { .. } } }` reaches each pair @@ -1020,6 +1066,24 @@ fn names_both_positions(first: &Form, second: &Form, left: u32, right: u32) -> b || (*first == Form::Local(right) && *second == Form::Local(left)) } +/// The bound a loop would have had if it did not stop one short. +/// +/// `n - 1` reads up to `n`. A length that is already a call — `v.len() - 1` — +/// gives back `v.len()`, which is what makes the extent the whole sequence +/// rather than a slice of it. +fn one_more_than(form: &Form) -> Option
{ + match form { + Form::Binary { + operator, + left, + right, + } if operator == "-" && **right == Form::Number("1".to_owned()) => { + Some(left.as_ref().clone()) + } + _ => None, + } +} + /// Whether a form is one less than another. fn is_predecessor_of(form: &Form, of: &Form) -> bool { matches!(form, Form::Binary { operator, left, right } diff --git a/crates/infact-rust-behaviors/src/idioms.rs b/crates/infact-rust-behaviors/src/idioms.rs index 64e0797..18e18a7 100644 --- a/crates/infact-rust-behaviors/src/idioms.rs +++ b/crates/infact-rust-behaviors/src/idioms.rs @@ -53,28 +53,93 @@ pub enum IdiomRefusal { } /// An algorithm recognized in written-out form. -#[derive(Debug, Clone, PartialEq, Eq)] +/// +/// Each one is a walk over pairs that decides something, and they differ in +/// three ways only: which pairs the walk has to reach, what it asks of a pair, +/// and what has to hold before the API may be recommended in its place. Adding +/// one is answering those three questions, not writing another recognizer. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Idiom { /// Deciding that no two elements of one sequence are equal. AllDifferent, + /// Deciding that each element is no greater than the one after it. + IsSorted, } impl Idiom { + /// Every algorithm this recognizes. + pub const ALL: &'static [Self] = &[Self::AllDifferent, Self::IsSorted]; + /// The API this recommends, as a path into the package that offers it. pub const fn callable_path(&self) -> (&'static str, &'static str) { match self { Self::AllDifferent => ("itertools", "itertools::Itertools::all_unique"), + Self::IsSorted => ("core", "slice::is_sorted"), + } + } + + /// Which pairs a walk must reach to decide this. + /// + /// Distinctness is a question about every pair, and it does not matter how + /// often each is seen or in which order. Sortedness is a question about + /// NEIGHBOURS: the same test over every pair decides something much + /// stronger, so admitting the other coverages here would report a sortedness + /// API for code that checks far more than sortedness. + const fn coverages(&self) -> &'static [Coverage] { + match self { + Self::AllDifferent => &[Coverage::Once, Coverage::BothWays], + Self::IsSorted => &[Coverage::Adjacent], + } + } + + /// Whether the recommended API needs an allocator. + const fn allocates(&self) -> bool { + match self { + Self::AllDifferent => true, + Self::IsSorted => false, } } /// What the code needs of its elements to compute this at all. /// - /// The other half of an [`Condition::ElementBound`] comes from the catalog. + /// The other half of a [`Condition::ElementBound`] comes from the catalog. /// This half is a property of the written-out shape: comparing two elements - /// with `==` is all a pairwise distinctness check asks of them. + /// with `==` is all a distinctness check asks of them, and `>` is all a + /// sortedness check asks. const fn element_bound_of_the_code(&self) -> &'static str { match self { Self::AllDifferent => "PartialEq", + Self::IsSorted => "PartialOrd", + } + } + + /// Whether a test asks this idiom's question of a pair. + /// + /// Distinctness asks whether the two are equal, and equality reads the same + /// either way round. Sortedness asks whether the EARLIER one is greater, + /// and reading that backwards is the opposite question, so the order the + /// walk bound them in is part of the test. + fn tests_the_pair(&self, condition: &Form, left: u32, right: u32) -> bool { + let Form::Binary { + operator, + left: first, + right: second, + } = condition + else { + return false; + }; + let named = |form: &Form, index: u32| *form == Form::Local(index); + match self { + Self::AllDifferent => { + operator == "==" + && ((named(first, left) && named(second, right)) + || (named(first, right) && named(second, left))) + } + // `a > b` and `b < a` are one test written two ways. + Self::IsSorted => { + (operator == ">" && named(first, left) && named(second, right)) + || (operator == "<" && named(first, right) && named(second, left)) + } } } @@ -89,13 +154,18 @@ impl Idiom { /// The element bound is read off the catalog rather than written here. /// Naming a bound from memory would be a claim about a version of a library /// that nothing checked, and it is exactly the claim a reader is least able - /// to verify. + /// to verify. When the two sides agree there is nothing for a reader to + /// check, and saying so anyway is noise that makes the real conditions + /// harder to see. pub fn conditions(&self, callable: &ExternalCallable) -> Vec { let mut conditions = Vec::new(); - if let Some(requires) = element_bound(callable) { + let code_requires = self.element_bound_of_the_code(); + if let Some(requires) = element_bound(callable) + && requires != code_requires + { conditions.push(Condition::ElementBound { requires, - code_requires: self.element_bound_of_the_code().to_owned(), + code_requires: code_requires.to_owned(), }); } match self { @@ -104,6 +174,10 @@ impl Idiom { Condition::ComparisonObservable, Condition::SmallInputsFavourTheCode, ]), + // Reaching the answer costs the same either way — one pass, the + // same comparisons, no allocation — so the only gap is what the two + // do where the order runs out. + Self::IsSorted => conditions.push(Condition::IncomparableElements), } conditions } @@ -200,34 +274,30 @@ pub struct Recognized { pub shape: Form, } -/// Recognize an all-different check in a normalized function body. -/// -/// The shape is a walk over each pair of one sequence that, on finding two -/// equal elements, does something that does not depend on WHICH pair it found. -/// That is the whole claim: a walk like that computes exactly whether a -/// duplicate exists, which is what the recommended API answers. What the code -/// then does with the answer — return it, print it, set a flag — is the -/// caller's business and does not change what the loop computed. +/// Recognize one algorithm in a normalized function body. /// -/// Either coverage will do, and that is worth saying explicitly: a square -/// guarded loop reaches each pair twice, and every reaction accepted below is -/// idempotent — returning twice is returning, setting a flag to the same -/// constant twice is setting it. The reaction that is NOT idempotent is -/// counting, and counting is refused for its own reasons, so nothing here -/// depends on which spelling was written. +/// The shape is a walk over pairs of one sequence that, on a pair answering the +/// idiom's test, does something that does not depend on WHICH pair it found. +/// That is the whole claim: a walk like that computes exactly whether such a +/// pair exists, which is what the recommended API answers. What the code then +/// does with the answer — return it, print it, set a flag — is the caller's +/// business and does not change what the loop computed. /// -/// Returns the sequence the check is over, which is what a caller would put the -/// recommended call on. -pub fn all_different(form: &Form, context: Context) -> Result { +/// For distinctness either of the unordered coverages will do, and that is +/// worth saying explicitly: a square guarded loop reaches each pair twice, and +/// every reaction accepted below is idempotent — returning twice is returning, +/// setting a flag to the same constant twice is setting it. The reaction that +/// is NOT idempotent is counting, and counting is refused for its own reasons. +pub fn recognize(idiom: Idiom, form: &Form, context: Context) -> Result { // Why the nearest thing to the shape was not it. Finding no walk at all is // the uninformative answer, so anything else outranks it. let mut refusal = IdiomRefusal::NotThisShape; - for coverage in [Coverage::Once, Coverage::BothWays] { - let pattern = pairwise_decision(coverage); + for coverage in idiom.coverages() { + let pattern = pairwise_decision(*coverage); for resolved in form.resolve_all(&pattern) { - match decides_distinctness(&resolved) { + match decides(idiom, &resolved) { Ok(sequence) => { - if !context.can_allocate { + if idiom.allocates() && !context.can_allocate { return Err(IdiomRefusal::CannotAllocate); } let mut shape = pattern.clone(); @@ -244,13 +314,18 @@ pub fn all_different(form: &Form, context: Context) -> Result Result { + recognize(Idiom::AllDifferent, form, context) +} + +/// Whether one match of the pairwise shape decides the idiom's question. /// /// Every question here is asked of a piece the matcher handed over, and the two /// names are the subject's own — a pattern numbers its roles by its own /// counting, and asking whether the reaction mentions the pattern's `Local(0)` /// would be asking about the wrong function's names. -fn decides_distinctness(resolved: &Resolved) -> Result { +fn decides(idiom: Idiom, resolved: &Resolved) -> Result { let (Some(sequence), Some(condition), Some(consequence)) = (resolved.hole(0), resolved.hole(1), resolved.hole(2)) else { @@ -259,11 +334,11 @@ fn decides_distinctness(resolved: &Resolved) -> Result { let (Some(left), Some(right)) = (resolved.local(0), resolved.local(1)) else { return Err(IdiomRefusal::NotThisShape); }; - if !compares_the_pair(condition, left, right) { + if !idiom.tests_the_pair(condition, left, right) { return Err(IdiomRefusal::DecidesSomethingElse); } // Reading either element means the pair is being used for its content, and - // an API that answers only whether a duplicate exists cannot supply that. + // an API that answers only whether such a pair exists cannot supply that. if consequence.references_local(left) || consequence.references_local(right) { return Err(IdiomRefusal::EscapesWithAValue); } @@ -273,7 +348,7 @@ fn decides_distinctness(resolved: &Resolved) -> Result { Ok(sequence.clone()) } -/// Whether a reaction to an equal pair records only that one was found. +/// Whether a reaction to a matching pair records only that one was found. /// /// Two spellings, and between them they are how the check is written. Leaving /// the function ends the walk, so nothing after it runs and the duplicate has @@ -334,27 +409,6 @@ fn is_inert_beside_recording(step: &Form) -> bool { || kind.starts_with("macro:")) } -/// Whether a test asks whether the two elements of a pair are equal. -/// -/// Equality only. `<` over every pair is a sortedness check and `!=` is the -/// opposite question, and both would be told to use a distinctness API by a -/// test that merely looked for a comparison. -fn compares_the_pair(condition: &Form, left: u32, right: u32) -> bool { - let Form::Binary { - operator, - left: first, - right: second, - } = condition - else { - return false; - }; - if operator != "==" { - return false; - } - let named = |form: &Form, index: u32| *form == Form::Local(index); - (named(first, left) && named(second, right)) || (named(first, right) && named(second, left)) -} - #[cfg(test)] mod tests { use super::*; @@ -612,17 +666,148 @@ mod tests { ); } + /// A sortedness check asks its question of neighbours, in order. + #[test] + fn an_ordered_test_over_neighbours_is_is_sorted() { + let ordered = Form::Binary { + operator: ">".to_owned(), + left: Box::new(Form::Local(0)), + right: Box::new(Form::Local(1)), + }; + let form = adjacent(escaping(ordered, Form::Constant("false".to_owned()))); + assert_eq!( + recognize(Idiom::IsSorted, &form, allowed()).map(|found| found.sequence), + Ok(Form::Free(0)) + ); + } + + /// Reading the comparison backwards is the opposite question. + #[test] + fn a_reversed_ordering_is_not_is_sorted() { + let reversed = Form::Binary { + operator: ">".to_owned(), + left: Box::new(Form::Local(1)), + right: Box::new(Form::Local(0)), + }; + let form = adjacent(escaping(reversed, Form::Constant("false".to_owned()))); + assert_eq!( + recognize(Idiom::IsSorted, &form, allowed()), + Err(IdiomRefusal::DecidesSomethingElse) + ); + } + + /// Sortedness is a question about neighbours, not about every pair. + /// + /// `a > b` over every pair says every element is no greater than every + /// later one, which is far more than sortedness and would be a much + /// stronger claim to summarize as `is_sorted`. + #[test] + fn an_ordered_test_over_every_pair_is_not_is_sorted() { + let ordered = Form::Binary { + operator: ">".to_owned(), + left: Box::new(Form::Local(0)), + right: Box::new(Form::Local(1)), + }; + let form = pairwise(escaping(ordered, Form::Constant("false".to_owned()))); + assert_eq!( + recognize(Idiom::IsSorted, &form, allowed()), + Err(IdiomRefusal::NotThisShape) + ); + } + + /// Distinctness is not decided by a walk over neighbours. + #[test] + fn an_equality_test_over_neighbours_is_not_all_different() { + let form = adjacent(escaping(equal_pair(), Form::Constant("false".to_owned()))); + assert_eq!( + all_different(&form, allowed()), + Err(IdiomRefusal::NotThisShape) + ); + } + + /// A sortedness check allocates nothing, so a `const fn` may still have it. + #[test] + fn is_sorted_is_offered_where_nothing_may_allocate() { + let ordered = Form::Binary { + operator: ">".to_owned(), + left: Box::new(Form::Local(0)), + right: Box::new(Form::Local(1)), + }; + let form = adjacent(escaping(ordered, Form::Constant("false".to_owned()))); + let context = Context { + can_allocate: false, + }; + assert!(recognize(Idiom::IsSorted, &form, context).is_ok()); + } + + /// Where both sides need the same of the elements there is nothing to check. + #[test] + fn a_bound_the_code_already_needs_is_not_a_condition() { + let conditions = Idiom::IsSorted.conditions(&excerpted("slice-is-sorted")); + assert!( + !conditions + .iter() + .any(|condition| matches!(condition, Condition::ElementBound { .. })), + "{conditions:?}" + ); + assert_eq!(conditions, vec![Condition::IncomparableElements]); + } + + fn adjacent(body: Form) -> Form { + Form::Sequence(vec![ + Form::Pairwise { + sequence: Box::new(Form::Free(0)), + left: Box::new(Pattern::Binding(0)), + right: Box::new(Pattern::Binding(1)), + body: Box::new(body), + coverage: Coverage::Adjacent, + }, + Form::Constant("true".to_owned()), + ]) + } + + /// One catalogued callable, copied out of a generated catalog. + /// + /// The standard library's catalog is not committed — 26 MB of generated + /// JSON, reproducible from a rustup component — so a test that needs a real + /// signature reads the excerpt rather than a pack that may not be built. + fn excerpted(name: &str) -> ExternalCallable { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/catalog") + .join(format!("{name}.json")); + serde_json::from_slice(&std::fs::read(&path).expect("catalog excerpt")) + .expect("parsing the excerpt") + } + + /// A shipped catalog entry, read rather than written out. + fn catalogued(path: &str) -> ExternalCallable { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../infact-packs"); + for api in ["rust-itertools/api", "rust-std/api"] { + let Ok(entries) = std::fs::read_dir(root.join(api)) else { + continue; + }; + for entry in entries.flatten() { + let Ok(bytes) = std::fs::read(entry.path()) else { + continue; + }; + let Ok(catalog) = serde_json::from_slice::(&bytes) + else { + continue; + }; + if let Some(found) = catalog + .callables + .into_iter() + .find(|callable| callable.path == path) + { + return found; + } + } + } + panic!("{path} is in no shipped catalog"); + } + /// The catalog entry the recognizer points at, as it is shipped. fn all_unique() -> ExternalCallable { - let catalog = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) - .join("../../infact-packs/rust-itertools/api/itertools-0.15.0.json"); - let catalog: infact_core::ExternalCatalog = - serde_json::from_slice(&std::fs::read(catalog).expect("itertools catalog")) - .expect("parsing the catalog"); - catalog - .callables - .into_iter() - .find(|callable| callable.path == Idiom::AllDifferent.callable_path().1) - .expect("all_unique in the catalog") + catalogued(Idiom::AllDifferent.callable_path().1) } } diff --git a/crates/infact-rust-behaviors/src/lib.rs b/crates/infact-rust-behaviors/src/lib.rs index d3bf874..f2f1dc6 100644 --- a/crates/infact-rust-behaviors/src/lib.rs +++ b/crates/infact-rust-behaviors/src/lib.rs @@ -784,10 +784,24 @@ fn collect_idiom_matches( let context = idioms::Context { can_allocate: !function.is_const, }; - let Ok(walked) = idioms::all_different(candidate, context) else { + for idiom in idioms::Idiom::ALL { + collect_one_idiom(*idiom, file, function, candidate, context, catalogs, output)?; + } + Ok(()) +} + +fn collect_one_idiom( + idiom: idioms::Idiom, + file: &ParsedFile, + function: &infact_rust_normalize::NormalizedFunction, + candidate: &Form, + context: idioms::Context, + catalogs: &[ExternalCatalog], + output: &mut BTreeSet>, +) -> Result<()> { + let Ok(walked) = idioms::recognize(idiom, candidate, context) else { return Ok(()); }; - let idiom = idioms::Idiom::AllDifferent; let (package, path) = idiom.callable_path(); // The callable has to be present AND still answer the question the idiom // decides. A catalog is generated data and a path is not a promise: the diff --git a/crates/infact-rust-behaviors/tests/fixtures/catalog/README.md b/crates/infact-rust-behaviors/tests/fixtures/catalog/README.md new file mode 100644 index 0000000..913a834 --- /dev/null +++ b/crates/infact-rust-behaviors/tests/fixtures/catalog/README.md @@ -0,0 +1,15 @@ +# Catalog fixtures + +One catalogued callable each, copied verbatim out of a generated catalog. + +The standard library's catalog is not committed — it is 26 MB of generated JSON, +reproducible in two seconds from a rustup component, and +`infact-packs/rust-std/README.md` says how. A test that needs to read a real +signature therefore reads it from here rather than from a pack that may not have +been built yet. + +They are excerpts, not catalogs: a single `ExternalCallable`, with no digest or +version around it, because a digest over one callable would claim to be the +digest of the library it came from. + + slice-is-sorted.json core 1.100.0-nightly, rustdoc format 61 diff --git a/crates/infact-rust-behaviors/tests/fixtures/catalog/slice-is-sorted.json b/crates/infact-rust-behaviors/tests/fixtures/catalog/slice-is-sorted.json new file mode 100644 index 0000000..cba7f5d --- /dev/null +++ b/crates/infact-rust-behaviors/tests/fixtures/catalog/slice-is-sorted.json @@ -0,0 +1,46 @@ +{ + "path": "slice::is_sorted", + "container": { + "type": { + "path": "slice" + } + }, + "signature": { + "inputs": [ + { + "name": "self", + "ty": { + "reference": { + "mutable": false, + "inner": { + "generic": { + "name": "Self" + } + } + } + } + } + ], + "output": { + "primitive": { + "name": "bool" + } + }, + "requirements": [ + { + "subject": { + "generic": { + "name": "T" + } + }, + "bounds": [ + { + "trait": { + "path": "PartialOrd" + } + } + ] + } + ] + } +} diff --git a/crates/infact-rust-normalize/examples/lower.rs b/crates/infact-rust-normalize/examples/lower.rs index f8c7ca1..34ee269 100644 --- a/crates/infact-rust-normalize/examples/lower.rs +++ b/crates/infact-rust-normalize/examples/lower.rs @@ -163,6 +163,9 @@ fn lower(form: &Form, level: usize, guesses: &Guesses, names: &Names) -> String Coverage::Once => "tuple_combinations()", // Each pair both ways round is what `permutations(2)` yields. Coverage::BothWays => "permutations(2)", + // `windows(2)` yields slices, not pairs; `tuple_windows` yields + // the pair this form holds. + Coverage::Adjacent => "tuple_windows()", }; format!( "for ({}, {}) in {}.{call} {{\n{}{}\n{}}}", diff --git a/crates/infact-rust-normalize/tests/normalize.rs b/crates/infact-rust-normalize/tests/normalize.rs index e618032..f744efb 100644 --- a/crates/infact-rust-normalize/tests/normalize.rs +++ b/crates/infact-rust-normalize/tests/normalize.rs @@ -748,3 +748,77 @@ fn a_lower_triangle_that_reaches_below_the_start_is_refused() { ); assert!(!form.contains("(pairwise"), "{form}"); } + +/// A loop reading each element and its neighbour is a walk over adjacent pairs. +#[test] +fn a_neighbour_loop_is_an_adjacent_pairwise_walk() { + let form = behavior_of( + "fn f(values: &[i32]) -> bool { + for i in 0..values.len() - 1 { + if values[i] > values[i + 1] { return false; } + } + true + }", + "f", + ); + assert!(form.contains("(pairwise-adjacent"), "{form}"); +} + +/// Reading a neighbour two along is not a walk over adjacent pairs. +#[test] +fn a_loop_reading_two_along_is_not_adjacent() { + let form = behavior_of( + "fn f(values: &[i32]) -> bool { + for i in 0..values.len() - 2 { + if values[i] > values[i + 2] { return false; } + } + true + }", + "f", + ); + assert!(!form.contains("(pairwise"), "{form}"); +} + +/// A loop that runs to the end reads past it, so it is a different walk. +/// +/// `for i in 0..v.len()` with `v[i + 1]` panics on the last step; treating it +/// as the `windows(2)` walk would report a working API for code that does not +/// work. +#[test] +fn a_neighbour_loop_that_runs_to_the_end_is_not_adjacent() { + let form = behavior_of( + "fn f(values: &[i32]) -> bool { + for i in 0..values.len() { + if values[i] > values[i + 1] { return false; } + } + true + }", + "f", + ); + assert!(!form.contains("(pairwise"), "{form}"); +} + +/// Adjacent pairs are not the same walk as every pair. +/// +/// The same test means something much weaker over neighbours than over all +/// pairs, so the two must not reduce to one form. +#[test] +fn adjacent_pairs_differ_from_every_pair() { + let adjacent = behavior_of( + "fn f(values: &[i32]) -> bool { + for i in 0..values.len() - 1 { if values[i] > values[i + 1] { return false; } } + true + }", + "f", + ); + let every = behavior_of( + "fn f(values: &[i32]) -> bool { + for i in 0..values.len() { + for j in i + 1..values.len() { if values[i] > values[j] { return false; } } + } + true + }", + "f", + ); + assert_ne!(adjacent, every); +} diff --git a/infact-packs/rust-std/README.md b/infact-packs/rust-std/README.md new file mode 100644 index 0000000..175320a --- /dev/null +++ b/infact-packs/rust-std/README.md @@ -0,0 +1,26 @@ +# rust-std + +Callable signatures for the Rust standard library, built from the rustdoc JSON +that ships as a rustup component rather than from a checkout of the compiler: + +```sh +rustup component add rust-docs-json --toolchain nightly +J=$(rustc +nightly --print sysroot)/share/doc/rust/json +V=$(rustc +nightly --version | awk '{print $2}') +infact catalog "$J/core.json" --package core --version "$V" \ + --output "infact-packs/rust-std/api/core-$V.json" +``` + +`alloc.json` and `std.json` build the same way and are not kept here, because +nothing yet matches against anything they add. Regenerating one takes two +seconds; carrying it does not. + +Only a nightly toolchain emits rustdoc JSON, so the version this records is a +nightly version. That is what the finding is bound to, and it is honest: the +signature a recommendation was checked against came from that compiler and no +other. + +`core.json` is large — 26 MB against itertools' 312 KB — because a language's +standard library is large and because rustdoc emits every item, including ones +no caller can name. A catalog that dropped them would be smaller and would be +asserting a visibility rule nothing here has checked. diff --git a/infact.toml b/infact.toml index 4a6ee1d..456fe6b 100644 --- a/infact.toml +++ b/infact.toml @@ -2,7 +2,7 @@ search-paths = ["../entl/parser-packs"] [catalogs] -search-paths = ["infact-packs/rust-itertools/api"] +search-paths = ["infact-packs/rust-itertools/api", "infact-packs/rust-std/api"] [behaviors] search-paths = ["infact-packs/rust-itertools/behaviors"]