autocxx icon indicating copy to clipboard operation
autocxx copied to clipboard

`shared_ptr<const int>` leads to C++ code that doesn't compile

Open bsilver8192 opened this issue 3 years ago • 4 comments

Expected Behavior

Emit bindings that work. dtolnay/cxx#1006 says that inserting a typedef results in cxx::bridge handling it already. dtolnay/cxx#850 talks about making it easier.

dtolnay/cxx#537 and dtolnay/cxx#885 have some discussions around making this type more useful, but that seems like a secondary problem.

Actual Behavior

C++ code with shared_ptr<const int> produces generated C++ that doesn't compile because it tries using shared_ptr<int>. It looks like bindgen is stripping out the necessary information before autocxx has a chance to do anything about it :(.

Steps to Reproduce the Problem

#800 has a test case with int.

Opaque C++ types might need to be handled differently. My code actually uses absl::Span, which might have other issues that require a hand-written intermediate typedef regardless.

Specifications

  • Version: 90c2aa19c131c66ab96d4962157c3afe2c87fcb1
  • Platform: Linux

bsilver8192 avatar Feb 14 '22 07:02 bsilver8192

Bleah, not having any luck running that workaround through autocxx either. The typedef gets resolved which strips the const off, and then it has the same problem.

bsilver8192 avatar Feb 14 '22 08:02 bsilver8192

It feels to me as though the solution here is to fix https://github.com/dtolnay/cxx/issues/850, and probably the way to fix that is unfortunately to make a whole new equivalent to cxx::SharedPtr, e.g. cxx::SharedPtrConst.

The codebase I'm working with does not tend to use shared_ptr so this is not a very high priority for me - I'm unlikely to work on this.

Here's a proposed phasing for how this could be tackled.

  1. As you note, bindgen does not currently tell us the distinction between shared_ptr<int> and shared_ptr<const int>. This is obviously the first thing to fix. This is probably not something that non-autocxx users of bindgen care about, so we probably need to address this by adding another annotation to our forked bindgen (https://github.com/adetaylor/rust-bindgen). See CppSemanticAttributeCreator.
  2. We should record such attributes as we parse functions within autocxx in src/conversion/parse.
  3. We should detect this within type_converter::confirm_inner_type_is_acceptable_generic_payload, and raise an error, such that such APIs are ignored.
  4. Alternatively/subsequently, in the caller of that function, convert_type_path, we arrange to fall through to the else block for such cases, in which case we'd end up creating a new typedef in C++ automatically, something like std_shared_ptr_int_AutocxxConcrete. So that you'd at least be able to handle these as opaque types within Rust code, though there would be no way to get to the int.
  5. Once cxx has an equivalent of cxx::SharedPtrConst we can switch to using it in these cases.

adetaylor avatar Feb 14 '22 19:02 adetaylor

I think the same thing applies to other smart pointer types, like unique_ptr.

I agree that making another set of smart pointer types seems like the way to solve it. Or maybe cxx::SharedPtr/cxx::UniquePtr could use where clauses to implement the appropriate methods/traits for all types?

bsilver8192 avatar Feb 14 '22 20:02 bsilver8192

Documenting my workaround for anybody who sees this before it's fixed: I hid the shared_ptr behind a typedef and then used block! on it. For my use case, I don't actually need to call the methods which have this problem, but if I did I think that writing a C++ conversion wrapper would work (use the shared_ptr constructor that takes a shared_ptr of a different type to share the refcount with).

bsilver8192 avatar Feb 23 '22 10:02 bsilver8192