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

Refactoring: Move implementation to header #9

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

Conversation

kaoskorobase
Copy link
Contributor

[ Code review, please don't merge ;) ]

Hi Stephen,

I finally started to work on some refactorings you've mentioned previously in various forum and issue threads. My plan was to move the implementation in sodium.cpp to sodium.h (this PR), introduce phantom template parameters for light_ptr, stream_, cell_, node, making sure everything compiles and tests pass and then replace light_ptr by actual values. Then work on the node garbage collector.

Unfortunately I've run into a problem during step one: After moving unsafe_new_stream to sodium.h (be24489), the tests fail because the stream's cleanup action doesn't seem to be run anymore. I've played around with a few things in order to understand the problem, but without success.

When you find the time, could you maybe have a look in case you spot the problem right away? Any help appreciated!

Thanks and best,
Stefan

CMake uses separate build directory
release-sink-machinery takes too long.
@kaoskorobase
Copy link
Contributor Author

The tests from the PR run fine on Linux with gcc or clang from Nix (don't remember which versions). The tests also fail on OSX with clang from Nix, so it seems to be an OSX specific problem.

@the-real-blackh Do you have any input on this or suggestions on how to proceed? Thanks!

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.

1 participant