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 PyMem_Raw{New,Resize} convenience macros #127415

Closed
picnixz opened this issue Nov 29, 2024 · 6 comments
Closed

Add PyMem_Raw{New,Resize} convenience macros #127415

picnixz opened this issue Nov 29, 2024 · 6 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API type-feature A feature request or enhancement

Comments

@picnixz
Copy link
Contributor

picnixz commented Nov 29, 2024

Feature or enhancement

Proposal:

We have convenience macros for PyMem_New and PyMem_Resize to allocate memory for n objects of the given type without checking if n * sizeof(T) would overflow but we don't have any for the raw allocators. I suggest adding those two macros.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

@picnixz picnixz added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Nov 29, 2024
@picnixz picnixz self-assigned this Nov 29, 2024
@encukou
Copy link
Member

encukou commented Nov 29, 2024

It seems to me that they wouldn't be worth it. PyMem_RawCalloc(sizeof(T), n) isn't that much worse to type than PyMem_RawNew(T, n), and PyMem_Resize isn't very convenient if you want proper error handling.

@ZeroIntensity
Copy link
Member

Looking at the PR, I think the biggest difference would be that PyMem_RawNew doesn't zero-out the memory. I personally don't find that useful, but I guess some users that are gung-ho on performance might like it.

@picnixz
Copy link
Contributor Author

picnixz commented Nov 29, 2024

It isn't that much worse but it does use a different allocator function (one being calloc(), the other one being malloc()). In addition, IIRC, calloc() sets the memory to 0 while malloc() leaves area of memory uninitialized. For downstream users, I feel that it's better that they retain the semantics they want as well.

@ZeroIntensity
Copy link
Member

I do see some usage, but Petr is less easily convinced. Generally for C API changes, you need to bicker with the C API WG first: https://github.com/capi-workgroup/decisions/issues

@picnixz
Copy link
Contributor Author

picnixz commented Nov 29, 2024

Oh right. I'll move the discussion there (I tend to forget about the C API WG when the feature is small enough but I can try writing a more convincing proposal tomorrow)

@picnixz
Copy link
Contributor Author

picnixz commented Dec 5, 2024

Having failed to convince the C API WG (and because I was less and less convinced as well), I will not push for this feature for now.

@picnixz picnixz closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants