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

shared_ptr object creation is not working between swift/c++ interoperability. #67355

Open
asheeshsoni01 opened this issue Jul 18, 2023 · 11 comments
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift compiler The Swift compiler itself linker error swift 6.0

Comments

@asheeshsoni01
Copy link

Description
Creating a shared_ptr object in the swift file is not working. It is stated in the documentation that shared_ptr functionality is supported but it is not working.

C++ header file: Sample.h

#ifndef Sample_h
#define Sample_h

#include
#include

inline std::shared_ptr makeSharedData(int data) {
return std::make_shared (data);
}

class sample {
public:
sample(std::shared_ptr data);
int read_data();
void write_data(int data);
void ref_func(int& data);
private:
std::shared_ptr m_data;
};

#endif /* Sample_h */

Respective Sample.cpp file :

#include "Sample.h"

sample::sample(std::shared_ptr data) {
m_data = data;
}

int sample::read_data() {
return *m_data;
}

void sample::write_data(int data) {
*m_data = data;
}

void sample::ref_func(int& data) {
data = 100;
}

Swift File Contents : TestFile.swift

import Foundation

func connectCPP() -> Int32 {
//1. making use of the inline function in header file and calling that to get the shared_ptr object which in turns used to make the
// class object. This is throwing below error :
/* std::__1::__shared_ptr_emplace<int, std::__1::allocator>::__shared_ptr_emplace(), referenced from:
vtable for std::__1::__shared_ptr_emplace<int, std::__1::allocator> in TestFile.o
std::__1::__shared_ptr_emplace<int, std::__1::allocator>::
__shared_ptr_emplace(), referenced from:
vtable for std::__1::__shared_ptr_emplace<int, std::__1::allocator> in TestFile.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
*/
var obj = sample(makeSharedData(30))

//2. If I am trying to use make_shared directly in swift file, It is throwing error of symbol not found for make_shared and
// shared_ptr.

var data = std::make_shared<int>(10)
var obj1 = sample(data)
obj.write_data(12)
return obj.read_data()

}

If shared_ptr support is present in current version of interoperability, then it should be working fine without any error.

@asheeshsoni01 asheeshsoni01 added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Jul 18, 2023
@hyp hyp added the c++ interop Feature: Interoperability with C++ label Jul 18, 2023
@hyp hyp self-assigned this Jul 18, 2023
@ByunghoonKim
Copy link

  1. inline makes problem here. If you separate your function definition and declaration without using inline, your code compiles.
  2. The expression std::make_share<int> is not a thing on Swift-side.
// sample.h
#include <memory>

std::shared_ptr<int> makeSharedData(int data);

class sample {
public:
    sample(std::shared_ptr<int> data);
    int read_data();
    void write_data(int data);
    void ref_func(int& data);
private:
    std::shared_ptr<int> m_data;
};
// sample.cpp
std::shared_ptr<int> makeSharedData(int data) {
    return std::make_shared<int>(data);
}

sample::sample(std::shared_ptr<int> data) {
    m_data = data;
}

int sample::read_data() {
    return *m_data;
}

void sample::write_data(int data) {
    *m_data = data;
}

void sample::ref_func(int& data) {
    data = 100;
}
// Test.swift
func connectCPP() -> Int32 {
    var obj = sample(makeSharedData(30))
    obj.write_data(12)
    return obj.read_data()
}

@asheeshsoni01
Copy link
Author

Thanks.. It is working fine for me now.

Just a small question in addition to this:
Can we have this similar approach for unique_ptr as well?

@moretromain
Copy link

moretromain commented Jul 19, 2023

I'm chiming in into this issue, because there is something that I might not completely understand about the new swift<->c++ interop: although sharing std::shared_ptr (like in this example) or std::vector works, I find it still quite underwhelming and not really "interop" material, since all you can do is pass around shared_ptrs or vectors by value, that are necessarily owned or created by their C++ counterpart. Despite what the swift doc is saying (and that one is really good for once, so much that it might be overselling the feature at that point...).
So quick questions:

  1. For a shared_ptr, is there a way to access the underlying object from swift ? I'm making sure the pointee is a swift-compatible object, its API is available from swift, but I can't access the object from the shared_ptr itself, only the pointer's API (empty, get refcount etc.), quite useless...
  2. For a vector: maybe it used to work, but as of right now (swift 5.9, Xcode 15 beta 4), conversion between a simple swift::Array and a std::vector<uint8_t> doesn't work, from both languages, the vector isn't usable as a RandomAccessCollection from Swift (or anything else for that matter, nothing to use it efficiently), so is the Array on the C++ side (only a subset of the read-only API is actually exposed to C++)
    I've been reproducing the examples from the swift main doc, or from https://github.com/apple/swift-cmake-examples, and most of the stuff that's presented in either the WWDC video or the docs isn't actually working, and the CMake examples are not even compiling...not to mention trying to modify them to experiment with vector/shared_ptr or multi-arch configurations etc.
    I was thrilled to give a try to that brand new promising feature, but after a few days trying things and selling the promise to a few colleagues, I might say it's a bit underwhelming so far.
    I hope I misunderstood something and I'm doing it all wrong ! (I'm surprised there's no way to locally use a std::vector from swift to easily transform/use it into something else using some '.withUnsafeXXX' mechanism, I'm not a swift expert, still learning, but that would be the way to interact between types will keeping the unsafe zone quite localised that I would expect..)

@hyp
Copy link
Contributor

hyp commented Jul 20, 2023

Sorry to hear that @moretromain , let me try to respond to specific points:

I'm chiming in into this issue, because there is something that I might not completely understand about the new swift<->c++ interop: although sharing std::shared_ptr (like in this example) or std::vector works, I find it still quite underwhelming and not really "interop" material, since all you can do is pass around shared_ptrs or vectors by value, that are necessarily owned or created by their C++ counterpart. Despite what the swift doc is saying (and that one is really good for once, so much that it might be overselling the feature at that point...). So quick questions:

  1. For a shared_ptr, is there a way to access the underlying object from swift ? I'm making sure the pointee is a swift-compatible object, its API is available from swift, but I can't access the object from the shared_ptr itself, only the pointer's API (empty, get refcount etc.), quite useless...

You should be able to access fields and call the specific methods on the pointee, for instance:

ptr.pointee.callMethod()

The ergonomics for shared_ptr are not great at the moment, and we're going to be working on improving them. We will need some Swift language additions as well, specifically easier ways to borrow values: https://forums.swift.org/t/desired-swift-language-features-that-can-advance-the-state-of-c-interoperability-support/66144/9. We are planning to work on it though!

  1. For a vector: maybe it used to work, but as of right now (swift 5.9, Xcode 15 beta 4), conversion between a simple swift::Array and a std::vector<uint8_t> doesn't work, from both languages, the vector isn't usable as a RandomAccessCollection from Swift (or anything else for that matter, nothing to use it efficiently), so is the Array on the C++ side (only a subset of the read-only API is actually exposed to C++)

It sounds like you're hitting #67410 . We're investigating it right now. You should be able to work around it by manually conforming a specific vector type you need to CxxRandomAccessCollection.

Once it's conformed you should be able to convert it to array by just going Array(vector), but that conformance is required first unfortunately.

I've been reproducing the examples from the swift main doc, or from https://github.com/apple/swift-cmake-examples, and most of the stuff that's presented in either the WWDC video or the docs isn't actually working, and the CMake examples are not even compiling...not to mention trying to modify them to experiment with vector/shared_ptr or multi-arch configurations etc.

Could you file an issue that describes the CMake examples build errors you're seeing to https://github.com/apple/swift-cmake-examples and CC @etcwilde on it.

I was thrilled to give a try to that brand new promising feature, but after a few days trying things and selling the promise to a few colleagues, I might say it's a bit underwhelming so far.
I hope I misunderstood something and I'm doing it all wrong ! (I'm surprised there's no way to locally use a std::vector from swift to easily transform/use it into something else using some '.withUnsafeXXX' mechanism, I'm not a swift expert, still learning, but that would be the way to interact between types will keeping the unsafe zone quite localised that I would expect..)

@etcwilde
Copy link
Contributor

@moretromain, yes, please do let me know what you're seeing, what Swift version you have, and what CMake version. Thanks!

@hborla hborla removed the triage needed This issue needs more specific labels label Apr 27, 2024
@redcode
Copy link

redcode commented Apr 29, 2024

You should be able to access fields and call the specific methods on the pointee, for instance:

ptr.pointee.callMethod()

Actually it is not as simple as you say. It is necessary for the class of the underlying managed object to have the SWIFT_IMMORTAL_REFERENCE attribute; otherwise, it will be freed after being accessed via .pointee, which in practice is a serious problem when you need to use a class from an external library to which you can't assign that attribute.

Minimal example

CppObject.hpp

#include <memory>

struct CppObject {
	using Shared = std::shared_ptr<CppObject>;
	char *string;

	CppObject(int size) noexcept;
	~CppObject();
	static Shared make_shared(int size);
	void print() const noexcept;
};

CppObject.cpp

#include "CppObject.hpp"
#include <cstring>
#include <iostream>


CppObject::CppObject(int size) noexcept {
	(string = new char[size + 1])[size] = '\0';
	std::memset(string, 'x', size);
}


CppObject::~CppObject() {
	delete[] string;
}


CppObject::Shared CppObject::make_shared(int size) {
	return std::make_shared<CppObject>(size);
}


void CppObject::print() const noexcept {
	std::cout << string;
}

Swift

let test: CppObject.Shared = CppObject.make_shared(256)
test.pointee.print()

Result

imagen
imagen

Undesirable fix

#include <swift/bridging>
/* ... */

struct CppObject {
	/* ... */
} SWIFT_IMMORTAL_REFERENCE;

@AnthonyLatsis AnthonyLatsis added compiler The Swift compiler itself c++ to swift Feature → c++ interop: c++ to swift linker error swift 6.0 labels Apr 29, 2024
@ravikandhadai
Copy link
Contributor

ravikandhadai commented May 17, 2024

@redcode It looks like it works as expected at least as per the current model. Note that test.pointee returns a copy of the value stored in the shared_ptr: test (on stack). (The pointee property in Swift is different from the deference operator T& operator*() in that it doesn't return a reference to the pointee but a copy of the value.) The value gets destroyed at the end of main. The code desugars to:

let test: CppObject.Shared = CppObject.make_shared(256)
let temp: CppObject = test.pointee // test.pointee is copied into temp
temp.print()
// temp is destroyed here and the destructor of temp is called
// test.pointee is generally valid and usable here. However, in this case the "string" stored in CppObject is actually shared by all instances and therefore it gets destroyed when `temp` gets destroyed. If CppObject behaves like value type and deep copies the string, `test.pointee` would still be valid here.

This is not different from what happens in this case:

let x = CppObject(256)
x.print()
// destructor of 'x' is called here. 

@redcode
Copy link

redcode commented May 17, 2024

@redcode It looks like it works as expected. Note that test.pointee returns a copy of the value stored in the shared_ptr: test (on stack). (The pointee property in Swift is different from the deference operator T& operator*() in that it doesn't return a reference to the pointee but a copy of the value.)

The problem is that the object, in my example, is not trivially destructible. The copy of pointee deletes CppObject::string when destroyed, which is deleted again when test is destroyed at the end of the scope. Obviously, this could be fixed by using a copy constructor that would take care of copying CppObject::string, but that would be another undesirable fix, wouldn't be?

@ravikandhadai
Copy link
Contributor

ravikandhadai commented May 17, 2024

Obviously, this could be fixed by using a copy constructor that would take care of copying CppObject::string, but that would be another undesirable fix, wouldn't be?

yes, while my explanation was to reason about the behavior within the current semantics/model, there is certainly a lot of room to improve support for C++ smart pointers in Swift e.g. as highlighted here: https://forums.swift.org/t/desired-swift-language-features-that-can-advance-the-state-of-c-interoperability-support/66144/9

Also, since CppObject is actually a type with shared storage, I think it is quite reasonable to import it as a Swift class with SWIFT_IMMORTAL_REFERENCE annotation, as mentioned. API Notes can come in handy for adding these annotations to a type defined in an external library.

@redcode
Copy link

redcode commented May 17, 2024

API Notes can come in handy for adding these annotations to a type defined in an external library.

This is very interesting, I was not aware of this new Clang feature. It seems to be just what I need to solve a number of problems.

Thanks :)

@mrousavy
Copy link

I found two very interesting "issues" with shared_ptr:

  1. As y'all have noticed, .pointee unfortunately copies the instance T instead of borrowing it. I'm wondering - instead of marking the pointer T as SWIFT_IMMORTAL_REFERENCE, is there maybe a way to get pointee as a borrowing T in Swift to avoid that copy? 🤔
  2. Passing a shared_ptr to a C-style Swift function (like a "callback") causes a heap corruption because it seems to be destroyed twice. Filed a bug report here: Passing a C++ type to a C-style Swift function crashes: malloc: Heap corruption detected *** #78292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift compiler The Swift compiler itself linker error swift 6.0
Projects
None yet
Development

No branches or pull requests

10 participants