gh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Windows
#141672
Conversation
Include/exports.h
Outdated
|
|
||
| #if defined(_WIN32) || defined(__CYGWIN__) | ||
| #if defined(Py_ENABLE_SHARED) | ||
| #if !defined(Py_BUILD_CORE) || defined(Py_ENABLE_SHARED) |
There was a problem hiding this comment.
I'm not 100% sure about this... don't we need it for our own exports?
There was a problem hiding this comment.
Only for Py_ENABLE_SHARED -- see #99888 that added the #else.
There was a problem hiding this comment.
Ah, I see we do these checks again everywhere else we use the macros. No problem them.
|
TBH, I have no view. My knowledge of C is very out of date, so I wouldn't trust anything I might say anyway... |
|
The (Edit: admittedly we also aren't the people that it's designed to help either) |
|
Here's a version that removes some redundant defines (setting things to defaults). |
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. This change mostly moves code around, and just __declspec(dllexport) on Windows.
|
|
||
| #define PyMODINIT_FUNC _PyINIT_FUNC_DECLSPEC PyObject* | ||
| #define PyMODEXPORT_FUNC _PyINIT_FUNC_DECLSPEC PyModuleDef_Slot* | ||
| #ifndef PyMODINIT_FUNC |
There was a problem hiding this comment.
Supporting an already defined PyMODINIT_FUNC/PyMODEXPORT_FUNC is a new feature. Is it really worth it? I'm not against it, just curious.
There was a problem hiding this comment.
It's an escape hatch: if there's an unexpected problem with this PR, this can allow people to hotfix it without patching or waiting for a new release.
encukou
added
the
🔨 test-with-buildbots
bedevere-bot
removed
the
🔨 test-with-buildbots
6 participants
PyMODINIT_FUNCdoes not apply__declspec(dllexport)on Windows #141671