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

placement new no longer works for D #116

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

burner
Copy link

@burner burner commented Jul 23, 2019

So the current way of working with allocators does no longer work.
This PR pulls out the placement new's and delete's and the allocators.

So the current way of working with allocators does no longer work.
This PR pulls out the placement new's and delete's and the allocators.
@ariovistus
Copy link
Owner

don't remember the specifics, but I think removing malloc allocation from references.d is going to have some implications.
Let's see here, for every D object that is wrapped and used in python, a reference to it is stored in Container, as is a reference to the python object. this prevents D from garbage collecting something python still has a reference to, and possibly vice versa.
I don't think Container is malloc allocated to avoid gc references to objects exposed to D, that doesn't make sense to me.
looking at https://github.com/ariovistus/pyd/blob/master/infrastructure/pyd/class_wrap.d#L95, it might be some similar fiddly implementation issue.

@burner
Copy link
Author

burner commented Jul 25, 2019

how do you suggest we get pyd compiling with 2.087 then?

@ariovistus
Copy link
Owner

well, if it was an implementation issue, it was 6 years ago. might not be an issue any more. guess we can try it and see what happens.

@ariovistus ariovistus merged commit 5afe656 into ariovistus:master Jul 25, 2019
@burner
Copy link
Author

burner commented Jul 25, 2019

sounds like a plan, thanks.

Could we also get a new release

@ariovistus
Copy link
Owner

ya, we're a bit overdue for one of those

@ariovistus
Copy link
Owner

0.11.0 should be up now, let me know if it isn't somewhere

@burner
Copy link
Author

burner commented Jul 26, 2019

looks good so far, I'll let you know if there is something else.

Thanks

John-Colvin added a commit to John-Colvin/pyd that referenced this pull request Aug 23, 2019
@John-Colvin
Copy link
Contributor

@ariovistus I think your fears were justified, I am seeing strange segfaults since this change. See #119 for an alternative

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.

None yet

3 participants