Redo how init methods from superclasses are propagated to subclasses

This currently removes a few methods, but is strictly more correct, especially in the location of category methods.

Author: madsmtm

crates/header-translator/src/cache.rs

@@ -1,99 +1,22 @@
 use std::collections::BTreeMap;
 use std::mem;
 
-use crate::availability::Availability;
-use crate::config::{ClassData, Config};
+use crate::config::Config;
 use crate::file::File;
 use crate::id::ItemIdentifier;
 use crate::method::Method;
 use crate::output::Output;
 use crate::stmt::Stmt;
 
-#[derive(Debug, PartialEq, Clone)]
-struct MethodCache {
-    availability: Availability,
-    methods: Vec<Method>,
-    category: ItemIdentifier<Option<String>>,
-}
-
-#[derive(Debug, PartialEq, Clone, Default)]
-struct ClassCache {
-    /// Methods that should be duplicated onto any subclass.
-    to_emit: Vec<MethodCache>,
-    // We don't need availability here, since a superclass' availability
-    // should always be greater than the subclass'.
-}
-
-impl ClassCache {
-    fn all_methods_data(&self) -> impl Iterator<Item = (bool, &str)> {
-        self.to_emit
-            .iter()
-            .flat_map(|cache| cache.methods.iter().map(|m| m.id()))
-    }
-}
-
 /// A helper struct for doing global analysis on the output.
 #[derive(Debug, PartialEq, Clone)]
 pub struct Cache<'a> {
-    classes: BTreeMap<ItemIdentifier, ClassCache>,
     config: &'a Config,
 }
 
 impl<'a> Cache<'a> {
-    pub fn new(output: &Output, config: &'a Config) -> Self {
-        let mut classes: BTreeMap<_, ClassCache> = BTreeMap::new();
-
-        for (name, library) in &output.libraries {
-            let _span = debug_span!("library", name).entered();
-            for (name, file) in &library.files {
-                let _span = debug_span!("file", name).entered();
-                for stmt in &file.stmts {
-                    if let Some((cls, method_cache)) = Self::cache_stmt(stmt) {
-                        let cache = classes.entry(cls.clone()).or_default();
-                        cache.to_emit.push(method_cache);
-                    }
-                }
-            }
-        }
-
-        Self { classes, config }
-    }
-
-    fn cache_stmt(stmt: &Stmt) -> Option<(&ItemIdentifier, MethodCache)> {
-        if let Stmt::Methods {
-            cls,
-            generics: _,
-            category,
-            availability,
-            superclasses: _,
-            methods,
-            description,
-        } = stmt
-        {
-            let _span = debug_span!("Stmt::Methods", ?cls).entered();
-            let methods: Vec<Method> = methods
-                .iter()
-                .filter(|method| method.emit_on_subclasses())
-                .cloned()
-                .collect();
-            if methods.is_empty() {
-                return None;
-            }
-            if description.is_some() {
-                warn!(description, "description was set");
-            }
-            let category = category.clone().with_new_path(cls);
-            Some((
-                cls,
-                MethodCache {
-                    availability: availability.clone(),
-                    methods,
-                    category,
-                },
-            ))
-        } else {
-            None
-        }
+    pub fn new(_output: &Output, config: &'a Config) -> Self {
+        Self { config }
     }
 
     pub fn update(&self, output: &mut Output) {
@@ -145,70 +68,6 @@ impl<'a> Cache<'a> {
             }
         }
 
-        let mut new_stmts = Vec::new();
-        for stmt in &mut file.stmts {
-            #[allow(clippy::single_match)] // There will be others
-            match stmt {
-                Stmt::ClassDecl {
-                    id,
-                    generics,
-                    superclasses,
-                    ..
-                } => {
-                    let _span = debug_span!("Stmt::ClassDecl", ?id).entered();
-                    let data = self.config.class_data.get(&id.name);
-
-                    // Used for duplicate checking (sometimes the subclass
-                    // defines the same method that the superclass did).
-                    let mut seen_methods: Vec<_> = self
-                        .classes
-                        .get(id)
-                        .map(|cache| cache.all_methods_data())
-                        .into_iter()
-                        .flatten()
-                        .collect();
-
-                    for (superclass, _) in &*superclasses {
-                        if let Some(cache) = self.classes.get(superclass) {
-                            new_stmts.extend(cache.to_emit.iter().filter_map(|cache| {
-                                let methods: Vec<_> = cache
-                                    .methods
-                                    .iter()
-                                    .filter(|method| !seen_methods.contains(&method.id()))
-                                    .filter_map(|method| {
-                                        method.clone().update(ClassData::get_method_data(
-                                            data,
-                                            &method.fn_name,
-                                        ))
-                                    })
-                                    .collect();
-                                if methods.is_empty() {
-                                    return None;
-                                }
-
-                                Some(Stmt::Methods {
-                                    cls: id.clone(),
-                                    generics: generics.clone(),
-                                    category: cache.category.clone(),
-                                    availability: cache.availability.clone(),
-                                    superclasses: superclasses.clone(),
-                                    methods,
-                                    description: Some(format!(
-                                        "Methods declared on superclass `{}`",
-                                        superclass.name
-                                    )),
-                                })
-                            }));
-
-                            seen_methods.extend(cache.all_methods_data());
-                        }
-                    }
-                }
-                _ => {}
-            }
-        }
-        file.stmts.extend(new_stmts);
-
         // Fix up a few typedef + enum declarations
         let mut iter = mem::take(&mut file.stmts).into_iter().peekable();
         while let Some(stmt) = iter.next() {

crates/header-translator/src/config.rs

@@ -157,6 +157,17 @@ pub struct MethodData {
     pub mutating: Option<bool>,
 }
 
+impl MethodData {
+    pub(crate) fn merge_with_superclass(self, superclass: Self) -> Self {
+        Self {
+            // Only use `unsafe` from itself, never take if from the superclass
+            unsafe_: self.unsafe_,
+            skipped: self.skipped | superclass.skipped,
+            mutating: self.mutating.or(superclass.mutating),
+        }
+    }
+}
+
 #[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
 #[serde(deny_unknown_fields)]
 pub struct FnData {

crates/header-translator/src/main.rs

@@ -178,6 +178,7 @@ fn main() -> Result<(), BoxError> {
         }
         writeln!(cargo_toml, "]")?;
     }
+    drop(cargo_toml);
     drop(span);
 
     let _span = info_span!("formatting").entered();

crates/header-translator/src/method.rs

@@ -242,8 +242,8 @@ pub struct Method {
 
 impl Method {
     /// Value that uniquely identifies the method in a class.
-    pub fn id(&self) -> (bool, &str) {
-        (self.is_class, &self.selector)
+    pub fn id(&self) -> (bool, String) {
+        (self.is_class, self.selector.clone())
     }
 
     fn parent_type_data(entity: &Entity<'_>, context: &Context<'_>) -> (bool, bool) {
@@ -320,17 +320,6 @@ impl Method {
         }
     }
 
-    pub fn update(mut self, data: MethodData) -> Option<Self> {
-        if data.skipped {
-            return None;
-        }
-
-        self.mutating = data.mutating.unwrap_or(false);
-        self.safe = !data.unsafe_;
-
-        Some(self)
-    }
-
     pub fn visit_required_types(&self, mut f: impl FnMut(&ItemIdentifier)) {
         for (_, arg) in &self.arguments {
             arg.visit_required_types(&mut f);