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

Pass nonpod by value #16

Merged
merged 7 commits into from
Oct 10, 2020
Merged

Pass nonpod by value #16

merged 7 commits into from
Oct 10, 2020

Conversation

adetaylor
Copy link
Collaborator

When existing C++ APIs demand that a non-POD type be passed
by value (e.g. a std::string or a struct which is not trivial)
we would like those APIs to be callable from Rust, but by
passing a UniquePtr<T> instead.

That's what this PR does.

It's not yet clear that such an approach is sufficiently ergonomic for seamless cxx-style interop with existing Rust APIs. We'll see...

It relies on dtolnay/cxx#218 so we can't merge it yet.

When existing C++ APIs demand that a non-POD type be passed
by value (e.g. a std::string or a struct which is not trivial)
we would like those APIs to be callable from Rust, but by
passing a UniquePtr<T> instead.

That's what this PR does.

It relies on cxx PR #218 so we can't merge it yet.
@dtolnay
Copy link
Contributor

dtolnay commented Oct 10, 2020

dtolnay/cxx#218 (replacement implementation) published in cxx 0.5.1.

@adetaylor
Copy link
Collaborator Author

I saw! Exciting!

@adetaylor adetaylor marked this pull request as ready for review October 10, 2020 23:31
@adetaylor adetaylor merged commit 107e5d5 into main Oct 10, 2020
Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're going to want some verification of the placement of non-POD fields before emitting to Rust as public fields. As currently implemented I think this is producing Rust datastructures with field offsets not matching the C++ structures.

    let cxx = indoc! {"
        S::S(uint32_t i) : i(i) {}
    "};
    let hdr = indoc! {"
        #include <cstdint>
        #include <string>
        struct S {
            explicit S(uint32_t i);
            std::string s;
            uint32_t i;
        };
    "};
    let rs = quote! {
        assert_eq!(ffi::cxxbridge::S_make_unique(1).i, 1);
    };
    run_test(cxx, hdr, rs, &["S"], &[]);
STDERR:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `1`', /tmp/.tmpVbDzJ7/input.rs:1:96
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

@adetaylor
Copy link
Collaborator Author

Thanks - I'll reply on #18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants