From abdb9a8a854a3a5318b1597bc5eeb1b8392a36b9 Mon Sep 17 00:00:00 2001 From: Brian Silverman Date: Wed, 16 Mar 2022 22:24:46 -0700 Subject: [PATCH] Fix assignment operators for types with names with underscores This should also gracefully handle other reasons for ignoring assignment operator overloads. Fixes google/autocxx#923 --- .../analysis/fun/implicit_constructors.rs | 31 ++++++++++++++++--- engine/src/conversion/analysis/fun/mod.rs | 23 ++++++++------ integration-tests/tests/integration_test.rs | 23 +++++++++++++- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/engine/src/conversion/analysis/fun/implicit_constructors.rs b/engine/src/conversion/analysis/fun/implicit_constructors.rs index a0356ca1d..975afbca6 100644 --- a/engine/src/conversion/analysis/fun/implicit_constructors.rs +++ b/engine/src/conversion/analysis/fun/implicit_constructors.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::collections::{hash_map, HashMap}; +use std::collections::{hash_map, HashMap, HashSet}; use syn::Type; @@ -15,6 +15,8 @@ use crate::{ analysis::{depth_first::depth_first, pod::PodAnalysis, type_converter::TypeKind}, api::{Api, ApiName, CppVisibility, FuncToConvert, SpecialMemberKind}, apivec::ApiVec, + convert_error::ConvertErrorWithContext, + ConvertError, }, known_types::{known_types, KnownTypeConstructorDetails}, types::QualifiedName, @@ -173,7 +175,7 @@ enum ExplicitFound { pub(super) fn find_constructors_present( apis: &ApiVec, ) -> HashMap { - let explicits = find_explicit_items(apis); + let (explicits, unknown_types) = find_explicit_items(apis); // These contain all the classes we've seen so far with the relevant properties on their // constructors of each kind. We iterate via [`depth_first`], so analyzing later classes @@ -248,6 +250,7 @@ pub(super) fn find_constructors_present( // unique_ptrs etc. let items_found = if bases_items_found.len() != bases.len() || fields_items_found.len() != field_info.len() + || unknown_types.contains(&name.name) { let is_explicit = |kind: ExplicitKind| -> SpecialMemberFound { // TODO: For https://github.com/google/autocxx/issues/815, map @@ -532,7 +535,9 @@ pub(super) fn find_constructors_present( all_items_found } -fn find_explicit_items(apis: &ApiVec) -> HashMap { +fn find_explicit_items( + apis: &ApiVec, +) -> (HashMap, HashSet) { let mut result = HashMap::new(); let mut merge_fun = |ty: QualifiedName, kind: ExplicitKind, fun: &FuncToConvert| match result .entry(ExplicitType { ty, kind }) @@ -548,6 +553,7 @@ fn find_explicit_items(apis: &ApiVec) -> HashMap) -> HashMap) -> HashMap + { + unknown_types.insert(impl_for.clone()); + } Api::Function { analysis: FnAnalysis { @@ -635,7 +658,7 @@ fn find_explicit_items(apis: &ApiVec) -> HashMap (), } } - result + (result, unknown_types) } /// Returns the information for a given known type. diff --git a/engine/src/conversion/analysis/fun/mod.rs b/engine/src/conversion/analysis/fun/mod.rs index f22f7493b..775476b7e 100644 --- a/engine/src/conversion/analysis/fun/mod.rs +++ b/engine/src/conversion/analysis/fun/mod.rs @@ -1083,10 +1083,22 @@ impl<'a> FnAnalyzer<'a> { CppVisibility::Protected => false, CppVisibility::Public => true, }; - if matches!( + if let Some(problem) = bads.into_iter().next() { + match problem { + Ok(_) => panic!("No error in the error"), + Err(problem) => set_ignore_reason(problem), + } + } else if fun.unused_template_param { + // This indicates that bindgen essentially flaked out because templates + // were too complex. + set_ignore_reason(ConvertError::UnusedTemplateParam) + } else if matches!( fun.special_member, Some(SpecialMemberKind::AssignmentOperator) ) { + // Be careful with the order of this if-else tree. Anything above here means we won't + // treat it as an assignment operator, but anything below we still consider when + // deciding which other C++ special member functions are implicitly defined. set_ignore_reason(ConvertError::AssignmentOperator) } else if fun.references.rvalue_ref_return { set_ignore_reason(ConvertError::RValueReturn) @@ -1102,15 +1114,6 @@ impl<'a> FnAnalyzer<'a> { ) { set_ignore_reason(ConvertError::RValueParam) - } else if let Some(problem) = bads.into_iter().next() { - match problem { - Ok(_) => panic!("No error in the error"), - Err(problem) => set_ignore_reason(problem), - } - } else if fun.unused_template_param { - // This indicates that bindgen essentially flaked out because templates - // were too complex. - set_ignore_reason(ConvertError::UnusedTemplateParam) } else { match kind { FnKind::Method { diff --git a/integration-tests/tests/integration_test.rs b/integration-tests/tests/integration_test.rs index d298108be..d87606b19 100644 --- a/integration-tests/tests/integration_test.rs +++ b/integration-tests/tests/integration_test.rs @@ -4571,12 +4571,33 @@ fn test_double_underscores_ignored() { uint32_t get_a() const { return 2; } uint32_t a; }; + + struct __default { __default() = default; }; + struct __destructor { ~__destructor() = default; }; + struct __copy { __copy(const __copy&) = default; }; + struct __copy_operator { __copy_operator &operator=(const __copy_operator&) = default; }; + struct __move { __move(__move&&) = default; }; + struct __move_operator { __move_operator &operator=(const __move_operator&) = default; }; "}; let rs = quote! { let b = ffi::B::make_unique(); assert_eq!(b.get_a(), 2); }; - run_test("", hdr, rs, &["B"], &[]); + run_test( + "", + hdr, + rs, + &[ + "B", + "__default", + "__destructor", + "__copy", + "__copy_operator", + "__move", + "__move_operator", + ], + &[], + ); } // This test fails on Windows gnu but not on Windows msvc