Merge pull request #1180 from google/fix-815

Work towards handling `=default` functions. This ensures we track the presence of `=default` functions but we don't currently do the right thing with their analysis, so this has no user-facing benefits just yet.
google · Nov 5, 2022 · 0885061 · 0885061
2 parents b00c5cd + 1ee6da4
commit 0885061
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 14 deletions.
diff --git a/engine/src/conversion/analysis/allocators.rs b/engine/src/conversion/analysis/allocators.rs
@@ -12,7 +12,10 @@ use syn::{parse_quote, punctuated::Punctuated, token::Comma, FnArg, ReturnType};
 
 use crate::{
     conversion::{
-        api::{Api, ApiName, CppVisibility, FuncToConvert, Provenance, References, TraitSynthesis},
+        api::{
+            Api, ApiName, CppVisibility, DeletedOrDefaulted, FuncToConvert, Provenance, References,
+            TraitSynthesis,
+        },
         apivec::ApiVec,
     },
     types::{make_ident, QualifiedName},
@@ -86,7 +89,7 @@ fn create_alloc_and_free(ty_name: QualifiedName) -> impl Iterator<Item = Api<Pod
                     synthesized_this_type: None,
                     synthetic_cpp: Some((cpp_function_body, CppFunctionKind::Function)),
                     add_to_trait: Some(synthesis),
-                    is_deleted: false,
+                    is_deleted: DeletedOrDefaulted::Neither,
                     provenance: Provenance::SynthesizedOther,
                     variadic: false,
                 }),

diff --git a/engine/src/conversion/analysis/casts.rs b/engine/src/conversion/analysis/casts.rs
@@ -12,7 +12,10 @@ use syn::{parse_quote, FnArg};
 
 use crate::{
     conversion::{
-        api::{Api, ApiName, CastMutability, Provenance, References, TraitSynthesis},
+        api::{
+            Api, ApiName, CastMutability, DeletedOrDefaulted, Provenance, References,
+            TraitSynthesis,
+        },
         apivec::ApiVec,
     },
     types::{make_ident, QualifiedName},
@@ -112,7 +115,7 @@ fn create_cast(from: &QualifiedName, to: &QualifiedName, mutable: CastMutability
                 mutable,
             }),
             synthetic_cpp: Some((CppFunctionBody::Cast, CppFunctionKind::Function)),
-            is_deleted: false,
+            is_deleted: DeletedOrDefaulted::Neither,
             provenance: Provenance::SynthesizedOther,
             variadic: false,
         }),

diff --git a/engine/src/conversion/analysis/fun/implicit_constructors.rs b/engine/src/conversion/analysis/fun/implicit_constructors.rs
@@ -11,6 +11,7 @@ use indexmap::{map::Entry, set::IndexSet as HashSet};
 
 use syn::{Type, TypeArray};
 
+use crate::conversion::api::DeletedOrDefaulted;
 use crate::{
     conversion::{
         analysis::{depth_first::depth_first, pod::PodAnalysis, type_converter::TypeKind},
@@ -573,7 +574,7 @@ fn find_explicit_items(
         .entry(ExplicitType { ty, kind })
     {
         Entry::Vacant(entry) => {
-            entry.insert(if fun.is_deleted {
+            entry.insert(if matches!(fun.is_deleted, DeletedOrDefaulted::Deleted) {
                 ExplicitFound::Deleted
             } else {
                 ExplicitFound::UserDefined(fun.cpp_vis)

diff --git a/engine/src/conversion/analysis/fun/mod.rs b/engine/src/conversion/analysis/fun/mod.rs
@@ -19,9 +19,9 @@ use crate::{
             type_converter::{self, add_analysis, TypeConversionContext, TypeConverter},
         },
         api::{
-            ApiName, CastMutability, CppVisibility, FuncToConvert, NullPhase, Provenance,
-            References, SpecialMemberKind, SubclassName, TraitImplSignature, TraitSynthesis,
-            UnsafetyNeeded, Virtualness,
+            ApiName, CastMutability, CppVisibility, DeletedOrDefaulted, FuncToConvert, NullPhase,
+            Provenance, References, SpecialMemberKind, SubclassName, TraitImplSignature,
+            TraitSynthesis, UnsafetyNeeded, Virtualness,
         },
         apivec::ApiVec,
         convert_error::ErrorContext,
@@ -513,7 +513,7 @@ impl<'a> FnAnalyzer<'a> {
                     **fun,
                     FuncToConvert {
                         special_member: Some(SpecialMemberKind::Destructor),
-                        is_deleted: false,
+                        is_deleted: DeletedOrDefaulted::Neither | DeletedOrDefaulted::Defaulted,
                         cpp_vis: CppVisibility::Public,
                         ..
                     }
@@ -1127,7 +1127,7 @@ impl<'a> FnAnalyzer<'a> {
             set_ignore_reason(ConvertErrorFromCpp::AssignmentOperator)
         } else if fun.references.rvalue_ref_return {
             set_ignore_reason(ConvertErrorFromCpp::RValueReturn)
-        } else if fun.is_deleted {
+        } else if matches!(fun.is_deleted, DeletedOrDefaulted::Deleted) {
             set_ignore_reason(ConvertErrorFromCpp::Deleted)
         } else {
             match kind {
@@ -2104,7 +2104,7 @@ impl<'a> FnAnalyzer<'a> {
                         references,
                         original_name: None,
                         synthesized_this_type: None,
-                        is_deleted: false,
+                        is_deleted: DeletedOrDefaulted::Neither,
                         add_to_trait: None,
                         synthetic_cpp: None,
                         provenance: Provenance::SynthesizedOther,

diff --git a/engine/src/conversion/api.rs b/engine/src/conversion/api.rs
@@ -246,6 +246,14 @@ pub(crate) enum Provenance {
     SynthesizedSubclassConstructor(Box<SubclassConstructorDetails>),
 }
 
+/// Whether a function has =delete or =default
+#[derive(Clone, Copy)]
+pub(crate) enum DeletedOrDefaulted {
+    Neither,
+    Deleted,
+    Defaulted,
+}
+
 /// A C++ function for which we need to generate bindings, but haven't
 /// yet analyzed in depth. This is little more than a `ForeignItemFn`
 /// broken down into its constituent parts, plus some metadata from the
@@ -283,7 +291,8 @@ pub(crate) struct FuncToConvert {
     /// If Some, this function didn't really exist in the original
     /// C++ and instead we're synthesizing it.
     pub(crate) synthetic_cpp: Option<(CppFunctionBody, CppFunctionKind)>,
-    pub(crate) is_deleted: bool,
+    /// =delete
+    pub(crate) is_deleted: DeletedOrDefaulted,
 }
 
 /// Layers of analysis which may be applied to decorate each API.

diff --git a/engine/src/conversion/parse/bindgen_semantic_attributes.rs b/engine/src/conversion/parse/bindgen_semantic_attributes.rs
@@ -14,7 +14,7 @@ use syn::{
 };
 
 use crate::conversion::{
-    api::{CppVisibility, Layout, References, SpecialMemberKind, Virtualness},
+    api::{CppVisibility, DeletedOrDefaulted, Layout, References, SpecialMemberKind, Virtualness},
     convert_error::{ConvertErrorWithContext, ErrorContext},
     ConvertErrorFromCpp,
 };
@@ -98,6 +98,16 @@ impl BindgenSemanticAttributes {
         }
     }
 
+    pub(super) fn get_deleted_or_defaulted(&self) -> DeletedOrDefaulted {
+        if self.has_attr("deleted") {
+            DeletedOrDefaulted::Deleted
+        } else if self.has_attr("defaulted") {
+            DeletedOrDefaulted::Defaulted
+        } else {
+            DeletedOrDefaulted::Neither
+        }
+    }
+
     fn parse_if_present<T: Parse>(&self, annotation: &str) -> Option<T> {
         self.0
             .iter()

diff --git a/engine/src/conversion/parse/parse_foreign_mod.rs b/engine/src/conversion/parse/parse_foreign_mod.rs
@@ -86,7 +86,7 @@ impl ParseForeignMod {
                     original_name: annotations.get_original_name(),
                     synthesized_this_type: None,
                     add_to_trait: None,
-                    is_deleted: annotations.has_attr("deleted"),
+                    is_deleted: annotations.get_deleted_or_defaulted(),
                     synthetic_cpp: None,
                     variadic: item.sig.variadic.is_some(),
                 });

diff --git a/integration-tests/tests/integration_test.rs b/integration-tests/tests/integration_test.rs
@@ -9964,6 +9964,13 @@ fn test_implicit_constructor_rules() {
             void a() const {}
         };
 
+        struct AllExplicitlyDefaulted {
+            AllExplicitlyDefaulted() = default;
+            AllExplicitlyDefaulted(const AllExplicitlyDefaulted&) = default;
+            AllExplicitlyDefaulted(AllExplicitlyDefaulted&&) = default;
+            void a() const {};
+        };
+
         struct PublicDeleted {
             PublicDeleted() = delete;
             PublicDeleted(const PublicDeleted&) = delete;
@@ -10505,6 +10512,12 @@ fn test_implicit_constructor_rules() {
         test_movable![ffi::AllImplicitlyDefaulted];
         test_call_a![ffi::AllImplicitlyDefaulted];
 
+        test_constructible![ffi::AllExplicitlyDefaulted];
+        test_make_unique![ffi::AllExplicitlyDefaulted];
+        test_copyable![ffi::AllExplicitlyDefaulted];
+        test_movable![ffi::AllExplicitlyDefaulted];
+        test_call_a![ffi::AllExplicitlyDefaulted];
+
         test_call_a![ffi::PublicDeleted];
 
         test_copyable![ffi::PublicDeletedDefault];
@@ -10878,6 +10891,7 @@ fn test_implicit_constructor_rules() {
         rs,
         &[
             "AllImplicitlyDefaulted",
+            "AllExplicitlyDefaulted",
             "PublicDeleted",
             "PublicDeletedDefault",
             "PublicDeletedCopy",