Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't generate bindings for move constructors #458

Merged
merged 2 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions engine/src/conversion/analysis/fun/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,13 @@ impl<'a> FnAnalyzer<'a> {
return Err(contextualize_error(ConvertError::UnusedTemplateParam));
}

// Reject move constructors.
if Self::is_move_constructor(&fun) {
return Err(contextualize_error(
ConvertError::MoveConstructorUnsupported,
));
}

// Analyze the return type, just as we previously did for the
// parameters.
let mut return_analysis = if let FnKind::Method(ref self_ty, MethodKind::Constructor) = kind
Expand Down Expand Up @@ -818,6 +825,27 @@ impl<'a> FnAnalyzer<'a> {
.next()
}

fn get_bindgen_special_member_annotation(fun: &ForeignItemFn) -> Option<String> {
fun.attrs
.iter()
.filter_map(|a| {
if a.path.is_ident("bindgen_special_member") {
let r: Result<LitStr, syn::Error> = a.parse_args();
match r {
Ok(ls) => Some(ls.value()),
Err(_) => None,
}
} else {
None
}
})
.next()
}

fn is_move_constructor(fun: &ForeignItemFn) -> bool {
Self::get_bindgen_special_member_annotation(fun).map_or(false, |val| val == "move_ctor")
}

fn get_reference_parameters_and_return(fun: &ForeignItemFn) -> (HashSet<Ident>, bool) {
let mut ref_params = HashSet::new();
let mut ref_return = false;
Expand Down
2 changes: 2 additions & 0 deletions engine/src/conversion/convert_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub enum ConvertError {
TooManyUnderscores,
UnknownDependentType,
IgnoredDependent,
MoveConstructorUnsupported,
}

fn format_maybe_identifier(id: &Option<Ident>) -> String {
Expand Down Expand Up @@ -86,6 +87,7 @@ impl Display for ConvertError {
ConvertError::TooManyUnderscores => write!(f, "Names containing __ are reserved by C++ so not acceptable to cxx")?,
ConvertError::UnknownDependentType => write!(f, "This item relies on a type not known to autocxx.")?,
ConvertError::IgnoredDependent => write!(f, "This item depends on some other type which autocxx could not generate.")?,
ConvertError::MoveConstructorUnsupported => write!(f, "This is a move constructor, for which we currently cannot generate bindings.")?,
}
Ok(())
}
Expand Down
17 changes: 12 additions & 5 deletions engine/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5065,11 +5065,18 @@ fn test_deleted_function() {
}

#[test]
#[ignore] // https://github.com/google/autocxx/issues/426
fn test_implicitly_deleted_copy_constructor() {
// We shouldn't generate bindings for implicitly deleted functions.
// The test is successful if the bindings compile, i.e. if autocxx doesn't
// attempt to call the implicitly deleted copy constructor.
fn test_ignore_move_constructor() {
// Test that we don't generate bindings for move constructors and,
// specifically, don't erroneously import them as copy constructors.
// We used to do this because bindgen creates the same Rust signature for
// move constructors and for copy constructors.
// The way this tests works is a bit subtle. Declaring a move constructor
// causes the copy constructor to be implicitly deleted (unless it is]
// explicitly declared). If we erroneously tried to create a binding for the
// move constructor (because the signature led us to believe it is a copy
// constructor), the generated C++ code would therefore try to call the
// deleted copy constructor, which would result in a compile error.
// The test is therefore successful if the bindings compile.
let hdr = indoc! {"
class A {
public:
Expand Down