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

Use (void) for empty function parameters #8002

Merged
merged 2 commits into from
Sep 7, 2024
Merged

Conversation

Yay295
Copy link
Contributor

@Yay295 Yay295 commented Apr 22, 2024

I just happened to notice that these are the only three C functions that don't take any arguments and use () instead of (void) in their definition.

@Yay295
Copy link
Contributor Author

Yay295 commented Apr 22, 2024

Ah, it looks like the webp functions are like that to get rid of the warning. They should actually have two parameters to match the PyMethodDef declaration, but if those parameters were added there would be a warning due to them being unused.

@nulano
Copy link
Contributor

nulano commented Apr 22, 2024

but if those parameters were added there would be a warning due to them being unused.

Can't you just use unnamed parameters to get rid of the warning?

@Yay295
Copy link
Contributor Author

Yay295 commented Apr 23, 2024

It looks like the solution in other places is just to cast it to the right type.

@radarhere
Copy link
Member

What is the benefit of (void) over ()? It's just a stylistic choice right, to try and more clearly communicate that there are no arguments?

If this is the general agreement, then sure, go ahead - but to me, requiring casting is a step too far for just a style preference.

@Yay295
Copy link
Contributor Author

Yay295 commented Apr 27, 2024

Casting is already used for this reason quite a lot elsewhere already. By "other places" I meant "other places in Pillow"; I wasn't talking about other projects.

Pillow/src/_webp.c

Lines 530 to 536 in c250a44

static struct PyMethodDef _anim_decoder_methods[] = {
{"get_info", (PyCFunction)_anim_decoder_get_info, METH_NOARGS, "get_info"},
{"get_chunk", (PyCFunction)_anim_decoder_get_chunk, METH_VARARGS, "get_chunk"},
{"get_next", (PyCFunction)_anim_decoder_get_next, METH_NOARGS, "get_next"},
{"reset", (PyCFunction)_anim_decoder_reset, METH_NOARGS, "reset"},
{NULL, NULL} /* sentinel */
};

https://github.com/search?q=repo%3Apython-pillow%2FPillow+%28PyCFunction%29&type=code

@radarhere
Copy link
Member

Casting is already used for this reason

None of those castings involve functions that have (void) for their parameters.

@Yay295 Yay295 force-pushed the patch-1 branch 2 times, most recently from b21437d to 527ca4b Compare June 22, 2024 14:11
@Yay295 Yay295 force-pushed the patch-1 branch 3 times, most recently from 4791010 to 0668aab Compare June 28, 2024 20:29
@Yay295 Yay295 force-pushed the patch-1 branch 2 times, most recently from 12a0e8a to 636be4d Compare July 19, 2024 13:16
@Yay295 Yay295 force-pushed the patch-1 branch 2 times, most recently from f34a324 to 38bcdd9 Compare August 1, 2024 14:50
@Yay295
Copy link
Contributor Author

Yay295 commented Aug 14, 2024

The WEBP functions were removed in #8213, so it's just the one JPEG function now.

@radarhere
Copy link
Member

What is the benefit of (void) over ()? It's just a stylistic choice right, to try and more clearly communicate that there are no arguments?

@Yay295
Copy link
Contributor Author

Yay295 commented Aug 28, 2024

They technically have different meanings in C. func(void) means "this function does not take any arguments", while func() means "this function takes an undefined number of arguments of undefined types". I believe I saw this come up as a warning somewhere when compiling, but I don't remember where, or what flags I might have enabled.

@hugovk hugovk merged commit a838da7 into python-pillow:main Sep 7, 2024
48 of 49 checks passed
@Yay295 Yay295 deleted the patch-1 branch September 7, 2024 05:45
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.

4 participants