Skip to content

Commit

Permalink
Fix assignment operators for types with names with underscores
Browse files Browse the repository at this point in the history
This should also gracefully handle other reasons for ignoring assignment
operator overloads.

Fixes google#923
  • Loading branch information
bsilver8192 committed Mar 17, 2022
1 parent 5bb7488 commit abdb9a8
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 15 deletions.
31 changes: 27 additions & 4 deletions engine/src/conversion/analysis/fun/implicit_constructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -173,7 +175,7 @@ enum ExplicitFound {
pub(super) fn find_constructors_present(
apis: &ApiVec<FnPrePhase1>,
) -> HashMap<QualifiedName, ItemsFound> {
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -532,7 +535,9 @@ pub(super) fn find_constructors_present(
all_items_found
}

fn find_explicit_items(apis: &ApiVec<FnPrePhase1>) -> HashMap<ExplicitType, ExplicitFound> {
fn find_explicit_items(
apis: &ApiVec<FnPrePhase1>,
) -> (HashMap<ExplicitType, ExplicitFound>, HashSet<QualifiedName>) {
let mut result = HashMap::new();
let mut merge_fun = |ty: QualifiedName, kind: ExplicitKind, fun: &FuncToConvert| match result
.entry(ExplicitType { ty, kind })
Expand All @@ -548,13 +553,16 @@ fn find_explicit_items(apis: &ApiVec<FnPrePhase1>) -> HashMap<ExplicitType, Expl
entry.insert(ExplicitFound::Multiple);
}
};
let mut unknown_types = HashSet::new();
for api in apis.iter() {
match api {
Api::Function {
analysis:
FnAnalysis {
kind: FnKind::Method { impl_for, .. },
param_details,
ignore_reason:
Ok(()) | Err(ConvertErrorWithContext(ConvertError::AssignmentOperator, _)),
..
},
fun,
Expand Down Expand Up @@ -588,6 +596,21 @@ fn find_explicit_items(apis: &ApiVec<FnPrePhase1>) -> HashMap<ExplicitType, Expl
fun,
)
}
Api::Function {
analysis:
FnAnalysis {
kind: FnKind::Method { impl_for, .. },
..
},
fun,
..
} if matches!(
fun.special_member,
Some(SpecialMemberKind::AssignmentOperator)
) =>
{
unknown_types.insert(impl_for.clone());
}
Api::Function {
analysis:
FnAnalysis {
Expand Down Expand Up @@ -635,7 +658,7 @@ fn find_explicit_items(apis: &ApiVec<FnPrePhase1>) -> HashMap<ExplicitType, Expl
_ => (),
}
}
result
(result, unknown_types)
}

/// Returns the information for a given known type.
Expand Down
23 changes: 13 additions & 10 deletions engine/src/conversion/analysis/fun/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
23 changes: 22 additions & 1 deletion integration-tests/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit abdb9a8

Please sign in to comment.