Skip to content

bpo-35081: Remove Py_BUILD_CORE from datetime.h #10238

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

bpo-35081: Remove Py_BUILD_CORE from datetime.h #10238

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f7832f
bpo-35081: Remove Py_BUILD_CORE from datetime.h
vstinner Oct 30, 2018
1858252
Add #undef PyDateTime_TimeZone_UTC to C file
vstinner Oct 30, 2018
22abaee
Add a comment
vstinner Oct 30, 2018
8a11d06
PyDateTimeAPI variable must not be defined in _datetimemodule.c
vstinner Oct 31, 2018
a2750c6
Extend the scope of _PY_DATETIME_IMPL
vstinner Oct 31, 2018
69a455d
Update Include/datetime.h
pganssle Nov 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions Include/datetime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,26 +180,6 @@ typedef struct {
#define PyDateTime_CAPSULE_NAME "datetime.datetime_CAPI"


#ifdef Py_BUILD_CORE

/* Macros for type checking when building the Python core. */
#define PyDate_Check(op) PyObject_TypeCheck(op, &PyDateTime_DateType)
#define PyDate_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DateType)

#define PyDateTime_Check(op) PyObject_TypeCheck(op, &PyDateTime_DateTimeType)
#define PyDateTime_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DateTimeType)

#define PyTime_Check(op) PyObject_TypeCheck(op, &PyDateTime_TimeType)
#define PyTime_CheckExact(op) (Py_TYPE(op) == &PyDateTime_TimeType)

#define PyDelta_Check(op) PyObject_TypeCheck(op, &PyDateTime_DeltaType)
#define PyDelta_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DeltaType)

#define PyTZInfo_Check(op) PyObject_TypeCheck(op, &PyDateTime_TZInfoType)
#define PyTZInfo_CheckExact(op) (Py_TYPE(op) == &PyDateTime_TZInfoType)

#else

/* Define global variable for the C API and a macro for setting it. */
static PyDateTime_CAPI *PyDateTimeAPI = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable will be created in every compilation unit that includes "datetime.h".

Copy link
Member

@pganssle pganssle Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was that already a problem for the C API? this was a pre-existing part of the public API.

At the moment, within CPython, this is only included in _datetimemodule.c and _testcapimodule.c. The latter was, according to Victor, already being compiled without Py_BUILD_CORE.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it's a little bit strange to use static here. But my PR doesn't change that, and I don't see how to change the API to avoid it.

@serhiy-storchaka: Would you mind to open a new issue to track this issue?

Copy link
Member

@pganssle pganssle Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vstinner I think your PR does change one thing, which is that this static variable is now created even when Py_BUILD_CORE is on, right? I don't think that there are any major negative consequences, though, and I don't see a way around it without either changing the public interface or re-introducing Py_BUILD_CORE to this file, which I gather was the aim of this PR.


Expand Down Expand Up @@ -264,8 +244,6 @@ static PyDateTime_CAPI *PyDateTimeAPI = NULL;
PyDateTimeAPI->Date_FromTimestamp( \
(PyObject*) (PyDateTimeAPI->DateType), args)

#endif /* Py_BUILD_CORE */

#ifdef __cplusplus
}
#endif
Expand Down
37 changes: 29 additions & 8 deletions Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

#include "Python.h"
#include "datetime.h"
#include "structmember.h"

#include <time.h>
Expand All @@ -11,14 +12,34 @@
# include <winsock2.h> /* struct timeval */
#endif

/* Differentiate between building the core module and building extension
* modules.
*/
#ifndef Py_BUILD_CORE
#define Py_BUILD_CORE
#endif
#include "datetime.h"
#undef Py_BUILD_CORE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes no difference in this case, but it's somewhat interesting that in the old version, Py_BUILD_CORE was un-defined regardless of whether it was originally defined.

I think this means that if Py_BUILD_CORE is defined when compiling _datetimemodules.c, it will now remain so for the rest of the file, as opposed to the pre-PR version, where it is unconditionally flipped off after importing datetime.h. I don't see any other references to it in the file, though, so it should be safe to remove this.


#undef PyDate_Check
#undef PyDate_CheckExact
#define PyDate_Check(op) PyObject_TypeCheck(op, &PyDateTime_DateType)
#define PyDate_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DateType)

#undef PyDateTime_Check
#undef PyDateTime_CheckExact
#define PyDateTime_Check(op) PyObject_TypeCheck(op, &PyDateTime_DateTimeType)
#define PyDateTime_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DateTimeType)

#undef PyTime_Check
#undef PyTime_CheckExact
#define PyTime_Check(op) PyObject_TypeCheck(op, &PyDateTime_TimeType)
#define PyTime_CheckExact(op) (Py_TYPE(op) == &PyDateTime_TimeType)

#undef PyDelta_Check
#undef PyDelta_CheckExact
#define PyDelta_Check(op) PyObject_TypeCheck(op, &PyDateTime_DeltaType)
#define PyDelta_CheckExact(op) (Py_TYPE(op) == &PyDateTime_DeltaType)

#undef PyTZInfo_Check
#undef PyTZInfo_CheckExact
#define PyTZInfo_Check(op) PyObject_TypeCheck(op, &PyDateTime_TZInfoType)
#define PyTZInfo_CheckExact(op) (Py_TYPE(op) == &PyDateTime_TZInfoType)

#undef PyDateTime_TimeZone_UTC


/*[clinic input]
module datetime
Expand Down