Allow subclass!() of superclasses with protected destructors

engine/src/conversion/analysis/abstract_types.rs

@@ -11,7 +11,7 @@ use syn::{punctuated::Punctuated, token::Comma};
 
 use super::{
     fun::{
-        FnAnalysis, FnKind, FnPhase, FnPrePhase2, MethodKind, PodAndConstructorAnalysis,
+        FnAnalysis, FnKind, FnPhase, FnPrePhase3, MethodKind, PodAndConstructorAnalysis,
         TraitMethodKind,
     },
     pod::PodAnalysis,
@@ -63,7 +63,7 @@ impl Signature {
 }
 
 /// Spot types with pure virtual functions and mark them abstract.
-pub(crate) fn mark_types_abstract(apis: ApiVec<FnPrePhase2>) -> ApiVec<FnPrePhase2> {
+pub(crate) fn mark_types_abstract(apis: ApiVec<FnPrePhase3>) -> ApiVec<FnPrePhase3> {
     #[derive(Default, Debug, Clone)]
     struct ClassAbstractState {
         undefined: HashSet<Signature>,
@@ -234,6 +234,7 @@ pub(crate) fn discard_ignored_functions(apis: ApiVec<FnPhase>) -> ApiVec<FnPhase
         Api::struct_unchanged,
         Api::enum_unchanged,
         Api::typedef_unchanged,
+        Api::subclass_unchanged,
     );
     apis_new
 }

engine/src/conversion/analysis/constructor_deps.rs

@@ -19,7 +19,7 @@ use crate::{
 };
 
 use super::fun::{
-    FnAnalysis, FnKind, FnPhase, FnPrePhase2, PodAndConstructorAnalysis, PodAndDepAnalysis,
+    FnAnalysis, FnKind, FnPhase, FnPrePhase3, PodAndConstructorAnalysis, PodAndDepAnalysis,
     TraitMethodKind,
 };
 
@@ -28,7 +28,7 @@ use super::fun::{
 /// which will later be used as edges in the garbage collection, because
 /// typically any use of a type will require us to call its copy or move
 /// constructor. The same applies to its alloc/free functions.
-pub(crate) fn decorate_types_with_constructor_deps(apis: ApiVec<FnPrePhase2>) -> ApiVec<FnPhase> {
+pub(crate) fn decorate_types_with_constructor_deps(apis: ApiVec<FnPrePhase3>) -> ApiVec<FnPhase> {
     let mut constructors_and_allocators_by_type = find_important_constructors(&apis);
     let mut results = ApiVec::new();
     convert_apis(
@@ -40,6 +40,7 @@ pub(crate) fn decorate_types_with_constructor_deps(apis: ApiVec<FnPrePhase2>) ->
         },
         Api::enum_unchanged,
         Api::typedef_unchanged,
+        Api::subclass_unchanged,
     );
     results
 }
@@ -71,7 +72,7 @@ fn decorate_struct(
 }
 
 fn find_important_constructors(
-    apis: &ApiVec<FnPrePhase2>,
+    apis: &ApiVec<FnPrePhase3>,
 ) -> HashMap<QualifiedName, Vec<QualifiedName>> {
     let mut results: HashMap<QualifiedName, Vec<QualifiedName>> = HashMap::new();
     for api in apis.iter() {

engine/src/conversion/analysis/deps.rs

@@ -39,6 +39,7 @@ impl HasDependencies for Api<FnPrePhase1> {
             Api::Subclass {
                 name: _,
                 superclass,
+                ..
             } => Box::new(std::iter::once(superclass)),
             Api::RustSubclassFn { details, .. } => Box::new(details.dependencies.iter()),
             Api::RustFn { deps, .. } => Box::new(deps.iter()),
@@ -88,6 +89,7 @@ impl HasDependencies for Api<FnPhase> {
             Api::Subclass {
                 name: _,
                 superclass,
+                ..
             } => Box::new(std::iter::once(superclass)),
             Api::RustSubclassFn { details, .. } => Box::new(details.dependencies.iter()),
             Api::RustFn { deps, .. } => Box::new(deps.iter()),

engine/src/conversion/analysis/fun/mod.rs

@@ -219,6 +219,11 @@ pub(crate) struct PodAndConstructorAnalysis {
     pub(crate) constructors: PublicConstructors,
 }
 
+#[derive(std::fmt::Debug)]
+pub(crate) struct SubclassAnalysis {
+    pub(crate) superclass_destructor_visibility: Option<CppVisibility>,
+}
+
 /// An analysis phase where we've analyzed each function, but
 /// haven't yet determined which constructors/etc. belong to each type.
 #[derive(std::fmt::Debug)]
@@ -228,6 +233,7 @@ impl AnalysisPhase for FnPrePhase1 {
     type TypedefAnalysis = TypedefAnalysis;
     type StructAnalysis = PodAnalysis;
     type FunAnalysis = FnAnalysis;
+    type SubclassAnalysis = ();
 }
 
 /// An analysis phase where we've analyzed each function, and identified
@@ -239,6 +245,19 @@ impl AnalysisPhase for FnPrePhase2 {
     type TypedefAnalysis = TypedefAnalysis;
     type StructAnalysis = PodAndConstructorAnalysis;
     type FunAnalysis = FnAnalysis;
+    type SubclassAnalysis = ();
+}
+
+/// An analysis phase where we've annotated each subclass with its superclass's
+/// destructor visibility.
+#[derive(std::fmt::Debug)]
+pub(crate) struct FnPrePhase3;
+
+impl AnalysisPhase for FnPrePhase3 {
+    type TypedefAnalysis = TypedefAnalysis;
+    type StructAnalysis = PodAndConstructorAnalysis;
+    type FunAnalysis = FnAnalysis;
+    type SubclassAnalysis = SubclassAnalysis;
 }
 
 #[derive(Debug)]
@@ -273,6 +292,7 @@ impl AnalysisPhase for FnPhase {
     type TypedefAnalysis = TypedefAnalysis;
     type StructAnalysis = PodAndDepAnalysis;
     type FunAnalysis = FnAnalysis;
+    type SubclassAnalysis = SubclassAnalysis;
 }
 
 /// Whether to allow highly optimized calls because this is a simple Rust->C++ call,
@@ -307,7 +327,7 @@ impl<'a> FnAnalyzer<'a> {
         unsafe_policy: &'a UnsafePolicy,
         config: &'a IncludeCppConfig,
         force_wrapper_generation: bool,
-    ) -> ApiVec<FnPrePhase2> {
+    ) -> ApiVec<FnPrePhase3> {
         let mut me = Self {
             unsafe_policy,
             extra_apis: ApiVec::new(),
@@ -332,9 +352,10 @@ impl<'a> FnAnalyzer<'a> {
             Api::struct_unchanged,
             Api::enum_unchanged,
             Api::typedef_unchanged,
+            Api::subclass_unchanged,
         );
-        let mut results = me.add_constructors_present(results);
-        me.add_subclass_constructors(&mut results);
+        let results = me.add_constructors_present(results);
+        let mut results = me.add_subclass_constructors(results);
         results.extend(me.extra_apis.into_iter().map(add_analysis));
         results
     }
@@ -502,12 +523,12 @@ impl<'a> FnAnalyzer<'a> {
         }
     }
 
-    fn add_subclass_constructors(&mut self, apis: &mut ApiVec<FnPrePhase2>) {
+    fn add_subclass_constructors(&mut self, apis: ApiVec<FnPrePhase2>) -> ApiVec<FnPrePhase3> {
         let mut results = ApiVec::new();
 
-        // Pre-assemble a list of types with known destructors, to avoid having to
-        // do a O(n^2) nested loop.
-        let types_with_destructors: HashSet<_> = apis
+        // Pre-assemble a list of superclass destructor visibility, to avoid
+        // having to do a O(n^2) nested loop.
+        let destructor_visibility_by_class: HashMap<_, _> = apis
             .iter()
             .filter_map(|api| match api {
                 Api::Function {
@@ -523,16 +544,14 @@ impl<'a> FnAnalyzer<'a> {
                     FuncToConvert {
                         special_member: Some(SpecialMemberKind::Destructor),
                         is_deleted: None | Some(Explicitness::Defaulted),
-                        cpp_vis: CppVisibility::Public,
                         ..
                     }
                 ) =>
                 {
-                    Some(impl_for)
+                    Some((impl_for.clone(), fun.cpp_vis))
                 }
                 _ => None,
             })
-            .cloned()
             .collect();
 
         for api in apis.iter() {
@@ -552,8 +571,12 @@ impl<'a> FnAnalyzer<'a> {
             } = api
             {
                 // If we don't have an accessible destructor, then std::unique_ptr cannot be
-                // instantiated for this C++ type.
-                if !types_with_destructors.contains(sup) {
+                // instantiated for this C++ type. Both public and protected destructors are
+                // accessible from a subclass's default constructor.
+                if !matches!(
+                    destructor_visibility_by_class.get(sup),
+                    Some(CppVisibility::Public | CppVisibility::Protected)
+                ) {
                     continue;
                 }
 
@@ -571,7 +594,28 @@ impl<'a> FnAnalyzer<'a> {
                 }
             }
         }
-        apis.extend(results.into_iter());
+
+        convert_apis(
+            apis,
+            &mut results,
+            Api::fun_unchanged,
+            Api::struct_unchanged,
+            Api::enum_unchanged,
+            Api::typedef_unchanged,
+            |name, superclass, _| {
+                Ok(Box::new(std::iter::once(Api::Subclass {
+                    name,
+                    analysis: SubclassAnalysis {
+                        superclass_destructor_visibility: destructor_visibility_by_class
+                            .get(&superclass)
+                            .copied(),
+                    },
+                    superclass,
+                })))
+            },
+        );
+
+        results
     }
 
     /// Analyze a given function, and any permutations of that function which
@@ -2125,6 +2169,7 @@ impl<'a> FnAnalyzer<'a> {
             },
             Api::enum_unchanged,
             Api::typedef_unchanged,
+            Api::subclass_unchanged,
         );
         results
     }
@@ -2367,6 +2412,31 @@ impl HasFieldsAndBases for Api<FnPrePhase2> {
     }
 }
 
+impl HasFieldsAndBases for Api<FnPrePhase3> {
+    fn name(&self) -> &QualifiedName {
+        self.name()
+    }
+
+    fn field_and_base_deps(&self) -> Box<dyn Iterator<Item = &QualifiedName> + '_> {
+        match self {
+            Api::Struct {
+                analysis:
+                    PodAndConstructorAnalysis {
+                        pod:
+                            PodAnalysis {
+                                field_definition_deps,
+                                bases,
+                                ..
+                            },
+                        ..
+                    },
+                ..
+            } => Box::new(field_definition_deps.iter().chain(bases.iter())),
+            _ => Box::new(std::iter::empty()),
+        }
+    }
+}
+
 /// Stringify a function argument for diagnostics
 fn describe_arg(arg: &syn::FnArg) -> String {
     match arg {

engine/src/conversion/analysis/fun/subclass.rs

@@ -40,7 +40,10 @@ pub(super) fn subclasses_by_superclass(
     let mut subclasses_per_superclass: HashMap<QualifiedName, Vec<SubclassName>> = HashMap::new();
 
     for api in apis.iter() {
-        if let Api::Subclass { name, superclass } = api {
+        if let Api::Subclass {
+            name, superclass, ..
+        } = api
+        {
             subclasses_per_superclass
                 .entry(superclass.clone())
                 .or_default()

engine/src/conversion/analysis/name_check.rs

@@ -50,6 +50,7 @@ pub(crate) fn check_names(apis: ApiVec<FnPhase>) -> ApiVec<FnPhase> {
         Api::Subclass {
             name: SubclassName(ref name),
             ref superclass,
+            ..
         } => {
             validate_all_segments_ok_for_cxx(name.name.segment_iter())?;
             validate_all_segments_ok_for_cxx(superclass.segment_iter())?;

engine/src/conversion/analysis/pod/mod.rs

@@ -66,6 +66,7 @@ impl AnalysisPhase for PodPhase {
     type TypedefAnalysis = TypedefAnalysis;
     type StructAnalysis = PodAnalysis;
     type FunAnalysis = ();
+    type SubclassAnalysis = ();
 }
 
 /// In our set of APIs, work out which ones are safe to represent
@@ -103,6 +104,7 @@ pub(crate) fn analyze_pod_apis(
         },
         |name, item| analyze_enum(name, item, parse_callback_results),
         Api::typedef_unchanged,
+        Api::subclass_unchanged,
     );
     // Conceivably, the process of POD-analysing the first set of APIs could result
     // in us creating new APIs to concretize generic types.
@@ -125,6 +127,7 @@ pub(crate) fn analyze_pod_apis(
         },
         |name, item| analyze_enum(name, item, parse_callback_results),
         Api::typedef_unchanged,
+        Api::subclass_unchanged,
     );
     assert!(more_extra_apis.is_empty());
     Ok(results)

engine/src/conversion/analysis/tdef.rs

@@ -40,6 +40,7 @@ impl AnalysisPhase for TypedefPhase {
     type TypedefAnalysis = TypedefAnalysis;
     type StructAnalysis = ();
     type FunAnalysis = ();
+    type SubclassAnalysis = ();
 }
 
 #[allow(clippy::needless_collect)] // we need the extra collect because the closure borrows extra_apis
@@ -78,6 +79,7 @@ pub(crate) fn convert_typedef_targets(
                 },
             })))
         },
+        Api::subclass_unchanged,
     );
     results.extend(extra_apis.into_iter().map(add_analysis));
     results

engine/src/conversion/api.rs

@@ -193,6 +193,7 @@ pub(crate) trait AnalysisPhase: std::fmt::Debug {
     type TypedefAnalysis: std::fmt::Debug;
     type StructAnalysis: std::fmt::Debug;
     type FunAnalysis: std::fmt::Debug;
+    type SubclassAnalysis: std::fmt::Debug;
 }
 
 /// No analysis has been applied to this API.
@@ -203,6 +204,7 @@ impl AnalysisPhase for NullPhase {
     type TypedefAnalysis = ();
     type StructAnalysis = ();
     type FunAnalysis = ();
+    type SubclassAnalysis = ();
 }
 
 #[derive(Clone, Debug)]
@@ -449,6 +451,7 @@ pub(crate) enum Api<T: AnalysisPhase> {
     Subclass {
         name: SubclassName,
         superclass: QualifiedName,
+        analysis: T::SubclassAnalysis,
     },
     /// Contributions to the traits representing superclass methods that we might
     /// subclass in Rust.
@@ -610,4 +613,19 @@ impl<T: AnalysisPhase> Api<T> {
     {
         Ok(Box::new(std::iter::once(Api::Enum { name, item })))
     }
+
+    pub(crate) fn subclass_unchanged(
+        name: SubclassName,
+        superclass: QualifiedName,
+        analysis: T::SubclassAnalysis,
+    ) -> Result<Box<dyn Iterator<Item = Api<T>>>, ConvertErrorWithContext>
+    where
+        T: 'static,
+    {
+        Ok(Box::new(std::iter::once(Api::Subclass {
+            name,
+            superclass,
+            analysis,
+        })))
+    }
 }