Skip to content

[Feature] Preserve shared_ptrs for non-const-ref std::vector conversion #100

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jpfeuffer
Copy link
Contributor

Preserves the old PyClass instances and their memory and allocates the difference to the new size.
Also removes one copy, since it copies the data directly to the memory addresses of the old and new Python objects.

@jpfeuffer
Copy link
Contributor Author

@hroest

@hroest
Copy link
Collaborator

hroest commented Apr 3, 2021

@jpfeuffer this sounds good if that is the case we described, can you add a test to autowrap? also why does this add a ton of boost code?

@uweschmitt
Copy link
Collaborator

Can we add a switch to enable or disable C++11 features? I don't want to break other users code when they use older build chains in their projects.

@jpfeuffer
Copy link
Contributor Author

@uweschmitt Yes, I considered that already. Will do.
@hroest I am not sure if it solves all our problems but it should be a) a bit more efficient due to the moves and b) a bit more of the data will be at the same place when moving back to the original list locations. I will think about a good test. And the new boost code is for make_shared, which requires boost's own move "module".

@jpfeuffer
Copy link
Contributor Author

Oh, by the way, libcpp_move requires a fairly recent Cython. Are you okay with that?

@uweschmitt
Copy link
Collaborator

Oh, by the way, libcpp_move requires a fairly recent Cython. Are you okay with that?

Yes, a minimal version requirement can be specified in setup.py and thus upgrading autowrap will not break this. The C-compiler setup on the target machine is out of our hands.

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.

3 participants