Skip to content

MAINT: const correctness for the generalized ufunc C API#23847

Merged
seberg merged 1 commit intonumpy:mainfrom
lpsinger:ufuncobject-types-const
Jun 14, 2023
Merged

MAINT: const correctness for the generalized ufunc C API#23847
seberg merged 1 commit intonumpy:mainfrom
lpsinger:ufuncobject-types-const

Conversation

@lpsinger
Copy link
Contributor

Numpy's internals do not modify this field, and C extensions that call PyUFunc_FromFuncAndDataAndSignature and friends are likely to pass a (static) const char array for the types argument. Make this field const so that calling code does not see discarded qualifier warnings.

@seberg
Copy link
Member

seberg commented Jun 1, 2023

If we change it, this is also true for the data (at the right place)? I would prefer having a release note because C++ users might need to add a cast thinking this is ABI relevant (which it is not).
(Although now that we allow backcompat building that should be less relevant.)

@lpsinger
Copy link
Contributor Author

lpsinger commented Jun 1, 2023

data is a void** --- it's a totally opaque pointer. The Numpy API won't modify it, but the user-provided loop function might. So I think it would make sense to leave data as non-const.

@seberg
Copy link
Member

seberg commented Jun 2, 2023

But we promise not to modify it, especially in that function. And I am very sure it must be const, if it wasn't things would fail on current NumPy (the pointer, not its conten in theoryt). So, yes, I think void but void * const* should be right and not matter for C because its OK to cast from non-const to const?)

@lpsinger lpsinger force-pushed the ufuncobject-types-const branch 4 times, most recently from 169e61f to 5889605 Compare June 2, 2023 21:43
@lpsinger
Copy link
Contributor Author

lpsinger commented Jun 5, 2023

But we promise not to modify it, especially in that function. And I am very sure it must be const, if it wasn't things would fail on current NumPy (the pointer, not its conten in theoryt). So, yes, I think void but void * const* should be right and not matter for C because its OK to cast from non-const to const?)

OK, I agree. Done.

The Numpy C API's functions for constructing generalized ufuncs
(`PyUFunc_FromFuncAndData`, `PyUFunc_FromFuncAndDataAndSignature`,
`PyUFunc_FromFuncAndDataAndSignatureAndIdentity`) take `types` and
`data` arguments that are not modified by Numpy's internals. Like
the `name` and `doc` arguments, third-party Python extension modules
are likely to supply these arguments from static constants. The
`types` and `data` arguments are now const-correct: they are
declared as `const char *types` and `void *const *data`,
respectively. C code should not be affected, but C++ code may be.
@lpsinger lpsinger force-pushed the ufuncobject-types-const branch from 5889605 to 381b4b3 Compare June 5, 2023 15:33
@lpsinger lpsinger changed the title MAINT: make PyUFuncObject.types const MAINT: const correctness for the generalized ufunc C API Jun 5, 2023
@lpsinger
Copy link
Contributor Author

lpsinger commented Jun 5, 2023

I also added a changelog entry.

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Thanks, may slash the release notes down a bit (the main point I want to mention is really that C++ folks may have to cast the function).

Otherwise, just going to sit on it a bit I think, because I going in now might be confusing with an rc in progress.

@seberg
Copy link
Member

seberg commented Jun 14, 2023

Thanks, let me just put it in. Unlikely that anyone uses C++ in this context, hopefully any confusion will quickly be gone (plus any fixups will probably be necessary either way).

@seberg seberg merged commit 37ef3e1 into numpy:main Jun 14, 2023
@lpsinger lpsinger deleted the ufuncobject-types-const branch June 14, 2023 13:22
lpsinger added a commit to lpsinger/numpy that referenced this pull request Jun 14, 2023
The types stated in the documentation for the `PyUFunc_FromFuncAndData`
and `PyUFunc_FromFuncAndDataAndSignature` has been out of date
relative to the header files for a while.

After numpy#23847, bring the documentation up to date.
lpsinger added a commit to lpsinger/numpy that referenced this pull request Apr 8, 2024
When internally defining ufuncs, declare data and types as const
to take advantage of the const correctness of the API. See numpy#23847.
lpsinger added a commit to lpsinger/numpy that referenced this pull request Apr 9, 2024
When internally defining ufuncs, declare data and types as const
to take advantage of the const correctness of the API. See numpy#23847.
lpsinger added a commit to lpsinger/numpy that referenced this pull request Apr 9, 2024
When internally defining ufuncs, declare data and types as const
to take advantage of the const correctness of the API. See numpy#23847.
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.

2 participants