Repository navigation
bpo-39943: Fix MSVC warnings in sre extension - #20508
vstinner merged 1 commit into
Conversation
Sorry, something went wrong.
|
@petdance Your review here would be appreciated as well. |
Sorry, something went wrong.
There was a problem hiding this comment.
I'm not sure that it's ok to put a comment into a #define macro. Maybe convert the macro to a static inline function, or put the comment outside (before) the #define.
Sorry, something went wrong.
There was a problem hiding this comment.
Good point, this works but it might lead to weird situations like
/*
some_commented_out_code();
MACRO;
*/and then the comment suddenly gets stopped in the middle due to macro expansion.
Sorry, something went wrong.
|
Looks fine to me. Good catch on the comments-in-a-macro. |
Sorry, something went wrong.
|
I checked Windows x64 job and I confirm that this PR fix all compiler warnings in sre files. |
Sorry, something went wrong.
|
Thanks @ammaraskar for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Sorry, something went wrong.
|
GH-24856 is a backport of this pull request to the 3.9 branch. |
Sorry, something went wrong.
(cherry picked from commit 06e3a27) Co-authored-by: Ammar Askar <ammar@ammaraskar.com>
|
It needs to be backpoted to 3.9. |
Sorry, something went wrong.
|
See also bpo-43499. |
Sorry, something went wrong.
While the PyMem_Del change seems harmless enough, I'm a little bit uneasy about the memcpy one. This file is pretty old and unlikely to change in the future but it seems like it opens up the possibility of someone making a mistake like DATA_STACK_POP(state, 5, ...) and have it not caught by the type checker because of the explicit cast.
For that particular one we could guard the explicitly casted version with #ifdef _MSC_VER or something so it's harder to make that mistake and not have it be caught by the CI.
https://bugs.python.org/issue39943