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

Add tests for smart pointers passed by value #1240

Closed
Geod24 opened this issue Sep 28, 2020 · 2 comments
Closed

Add tests for smart pointers passed by value #1240

Geod24 opened this issue Sep 28, 2020 · 2 comments
Assignees
Labels
prio-high Will have an effect in the next three sprints Story-Points:8 This takes 5 to 7 days to complete type-enhancement An improvement of existing functionalities
Milestone

Comments

@Geod24
Copy link
Collaborator

Geod24 commented Sep 28, 2020

Currently our smart pointers bindings (std::shared_ptr & co) do not have a copy constructor defined. In order to avoid issues, we pass them by reference.

However, upstream stellar changed their SCP implementation and some smart pointers are passed by value. Hence, we need to add support for that in order for our bindings to be usable.

TODO:

  • Add the required copy constructors to our smart pointers, for all supported platforms;
  • Add a test here to ensure that pass-by-value works correctly, as well as returning. So C++ to D by value, D to C++ by value, return by value from C++ to D, return by value from D to C++.

Be wary of bugs.

@Geod24 Geod24 added prio-high Will have an effect in the next three sprints type-enhancement An improvement of existing functionalities labels Sep 28, 2020
@Geod24 Geod24 added this to the 2. Validator milestone Sep 28, 2020
@AndrejMitrovic AndrejMitrovic changed the title Add tests for smart pointers passed by values Add tests for smart pointers passed by value Oct 5, 2020
@Geod24
Copy link
Collaborator Author

Geod24 commented Oct 6, 2020

We're affected by https://issues.dlang.org/show_bug.cgi?id=20235
I'll raise the point in the D foundation meeting on Friday.

@bpalaggi bpalaggi added the Story-Points:8 This takes 5 to 7 days to complete label Oct 12, 2020
@Geod24
Copy link
Collaborator Author

Geod24 commented Oct 18, 2020

So the tests hasn't been checked in because they fail - @ferencdg found a bug that is a blocker for us. In that regardm I consider this issue closed, since it achieved its aim to verify if things worked or not.

@Geod24 Geod24 closed this as completed Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio-high Will have an effect in the next three sprints Story-Points:8 This takes 5 to 7 days to complete type-enhancement An improvement of existing functionalities
Projects
None yet
Development

No branches or pull requests

3 participants