Repository navigation
gh-149044: Fix PySlot_END macro for C++ - #158866
Conversation
Initialize all fields in the macro. Otherwise, g++ -Wall -Wextra complains that some fields are not initialized [-Werror=missing-field-initializers]: error: missing initializer for member ‘PySlot::sl_flags’ error: missing initializer for member ‘PySlot::<anonymous>’ error: missing initializer for member ‘PySlot::<anonymous>’
vstinner
added
topic-C-API
needs backport to 3.15
|
Note: I found this issue while working on adding support for the limited C API to pythoncapi-compat which builds its C/C++ extension with |
|
I created this PR to just fix I also prepared draft PR #158867 which enables |
| {.sl_id=(NAME), .sl_flags=PySlot_STATIC, .sl_ptr=(VALUE)} | ||
|
|
||
| #define PySlot_END {0} | ||
| #define PySlot_END {0, 0, {0}, {0}} |
There was a problem hiding this comment.
I'm not 100% sure that this syntax works on all C/C++ compilers. In C, it's common to use {0}. A more ugly alternative is to have a separated implementation for C++:
#ifdef __cplusplus
# define PySlot_END {0, 0, {0}, {0}}
#else
# define PySlot_END {0}
#endif
There was a problem hiding this comment.
Maybe use NULL?
| #define PySlot_END {0, 0, {0}, {0}} | |
| #define PySlot_END {0, 0, {0}, {NULL}} |
Not sure if compilers warn about this, but I heard that C++ is stricter about nullptr↔int casts.
There was a problem hiding this comment.
You might get away with
# define PySlot_END {}
for c++ (at least c++11 upwards I think). That's the idiomatic thing for "initialize everything to its default (likely 0) value". But maybe the explicit version with all members is better given it's a fairly short list of members.
|
I updated the branch to retrieve the MSan fix. |
|
cc @encukou |
|
@encukou @da-woods: I updated the PR to use @da-woods: Using |
|
Yeah I think a single implementation is probably cleaner. I don't believe |
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
Thank you!
({} is valid C! But not our C: it's new in C23...)
|
GH-158965 is a backport of this pull request to the 3.15 branch. |
bedevere-app
Bot
removed
the
needs backport to 3.15
Oh, C23 seems to be far in the future, we "just" moved to C11 for the internal C code :-) But it's great that the C standard is still evolving! Thanks for the review and the merge. Using |
3 participants
Initialize all fields in the macro. Otherwise, g++ -Wall -Wextra complains that some fields are not initialized
[-Werror=missing-field-initializers]:
error: missing initializer for member ‘PySlot::sl_flags’
error: missing initializer for member ‘PySlot::’
error: missing initializer for member ‘PySlot::’