gh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Windows#141672
gh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Windows#141672encukou merged 11 commits intopython:mainfrom
__declspec(dllexport) on Windows#141672Conversation
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). |
|
|
||
| #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.
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 5091b79 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141672%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
| #if defined(Py_BUILD_CORE) | ||
| #define _PyINIT_EXPORTED_SYMBOL Py_EXPORTED_SYMBOL | ||
| #else | ||
| #define _PyINIT_EXPORTED_SYMBOL __declspec(dllexport) |
There was a problem hiding this comment.
I think a better option instead is to define Py_EXPORTED_SYMBOL here and then use that.
Also I feel like an escape hatch like Py_EMBED_MODULE where it basically uses Py_LOCAL_SYMBOL instead could be an option as well for when one does not want the module init function exported due to intending to use PyImport_AppendInitTab within the exe that they directly build the module into.
There was a problem hiding this comment.
I don't want to change the meaning of Py_EXPORTED_SYMBOL.
For the inittab, you can avoid PyMODINIT_FUNC/PyMODEXPORT_FUNC, and just use a normal declaration.
PyMODINIT_FUNCdoes not apply__declspec(dllexport)on Windows #141671