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

gh-103092: Adapt _ctypes's PyCStgDict type to heap type #103932

Closed

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 27, 2023

Port PyCStgDict_Type to heap type.

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Apr 27, 2023

This probably fails because of the dict hacks in StructUnionType_new (wild guess).

@corona10 corona10 self-requested a review April 27, 2023 23:31
@kumaraditya303
Copy link
Contributor

kumaraditya303 commented May 30, 2023

While I am still trying to figure out where it breaks, you may proceed to port other simpler types (if there are any).

@vstinner
Copy link
Member

What's the status of this PR? Is it ready for review? It's still marked as a draft.

PyCStgDict_traverse(StgDictObject *self, visitproc visit, void *arg)
{
Py_VISIT(Py_TYPE(self));
Py_VISIT(self->proto);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an extra visit to the proto member? The CDataType_traverse function in _ctypes.c has an explicit Py_VISIT(dict->proto).

@erlend-aasland erlend-aasland changed the title gh-103092: Isolate _ctypes, part 2 gh-103092: Adapt _ctypes.PyCStgDict_Type to heap type Jan 9, 2024
@erlend-aasland erlend-aasland changed the title gh-103092: Adapt _ctypes.PyCStgDict_Type to heap type gh-103092: Adapt _ctypes's PyCStgDict type to heap type Jan 9, 2024
@erlend-aasland
Copy link
Contributor Author

I don't have the bandwidth to follow up this right now; abandoning this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants