Skip to content

Commit

Permalink
Merge pull request #927 from bsilver8192/assignment-operators-undersc…
Browse files Browse the repository at this point in the history
…ores

Fix assignment operators for types with names with underscores
  • Loading branch information
adetaylor authored Mar 17, 2022
2 parents 5bb7488 + abdb9a8 commit a34c133
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 a34c133

Please sign in to comment.