From b5ebbc5241a4800232e6ce1747021fb53056271e Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Sun, 10 Dec 2023 01:36:32 -0500 Subject: [PATCH 1/2] gh-89039: call subclass constructors in datetime.*.replace When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called. Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored. --- Lib/test/datetimetester.py | 62 ++++++++++++++-- ...3-12-18-20-10-50.gh-issue-89039.gqFdtU.rst | 6 ++ Modules/_datetimemodule.c | 72 +++++++++++++++---- 3 files changed, 120 insertions(+), 20 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-12-18-20-10-50.gh-issue-89039.gqFdtU.rst diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 53ad5e57ada017..9cba33dd6aee22 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -1723,11 +1723,24 @@ def test_replace(self): def test_subclass_replace(self): class DateSubclass(self.theclass): - pass + def __new__(cls, *args, **kwargs): + result = self.theclass.__new__(cls, *args, **kwargs) + result.extra = 7 + return result dt = DateSubclass(2012, 1, 1) - self.assertIs(type(dt.replace(year=2013)), DateSubclass) - self.assertIs(type(copy.replace(dt, year=2013)), DateSubclass) + + test_cases = [ + ('self.replace', dt.replace(year=2013)), + ('copy.replace', copy.replace(dt, year=2013)), + ] + + for name, res in test_cases: + with self.subTest(name): + self.assertIs(type(res), DateSubclass) + self.assertEqual(res.year, 2013) + self.assertEqual(res.month, 1) + self.assertEqual(res.extra, 7) def test_subclass_date(self): @@ -3025,6 +3038,26 @@ def __new__(cls, *args, **kwargs): self.assertIsInstance(dt, DateTimeSubclass) self.assertEqual(dt.extra, 7) + def test_subclass_replace_fold(self): + class DateTimeSubclass(self.theclass): + pass + + dt = DateTimeSubclass(2012, 1, 1) + dt2 = DateTimeSubclass(2012, 1, 1, fold=1) + + test_cases = [ + ('self.replace', dt.replace(year=2013), 0), + ('self.replace', dt2.replace(year=2013), 1), + ('copy.replace', copy.replace(dt, year=2013), 0), + ('copy.replace', copy.replace(dt2, year=2013), 1), + ] + + for name, res, fold in test_cases: + with self.subTest(name, fold=fold): + self.assertIs(type(res), DateTimeSubclass) + self.assertEqual(res.year, 2013) + self.assertEqual(res.fold, fold) + def test_fromisoformat_datetime(self): # Test that isoformat() is reversible base_dates = [ @@ -3705,11 +3738,28 @@ def test_replace(self): def test_subclass_replace(self): class TimeSubclass(self.theclass): - pass + def __new__(cls, *args, **kwargs): + result = self.theclass.__new__(cls, *args, **kwargs) + result.extra = 7 + return result ctime = TimeSubclass(12, 30) - self.assertIs(type(ctime.replace(hour=10)), TimeSubclass) - self.assertIs(type(copy.replace(ctime, hour=10)), TimeSubclass) + ctime2 = TimeSubclass(12, 30, fold=1) + + test_cases = [ + ('self.replace', ctime.replace(hour=10), 0), + ('self.replace', ctime2.replace(hour=10), 1), + ('copy.replace', copy.replace(ctime, hour=10), 0), + ('copy.replace', copy.replace(ctime2, hour=10), 1), + ] + + for name, res, fold in test_cases: + with self.subTest(name, fold=fold): + self.assertIs(type(res), TimeSubclass) + self.assertEqual(res.hour, 10) + self.assertEqual(res.minute, 30) + self.assertEqual(res.extra, 7) + self.assertEqual(res.fold, fold) def test_subclass_time(self): diff --git a/Misc/NEWS.d/next/Library/2023-12-18-20-10-50.gh-issue-89039.gqFdtU.rst b/Misc/NEWS.d/next/Library/2023-12-18-20-10-50.gh-issue-89039.gqFdtU.rst new file mode 100644 index 00000000000000..d1998d75e9fd76 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-18-20-10-50.gh-issue-89039.gqFdtU.rst @@ -0,0 +1,6 @@ +When replace() method is called on a subclass of datetime, date or time, +properly call derived constructor. Previously, only the base class's +constructor was called. + +Also, make sure to pass non-zero fold values when creating subclasses in +various methods. Previously, fold was silently ignored. diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 9b8e0a719d9048..a9f1a8d4fe5bcc 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -1045,6 +1045,39 @@ new_datetime_ex(int year, int month, int day, int hour, int minute, new_datetime_ex2(y, m, d, hh, mm, ss, us, tzinfo, fold, \ &PyDateTime_DateTimeType) +static PyObject * +call_subclass_fold(PyObject *cls, int fold, const char *format, ...) { + PyObject *kwargs = NULL, *res = NULL; + va_list va; + + va_start(va, format); + PyObject *args = Py_VaBuildValue(format, va); + va_end(va); + if (args == NULL) { + return NULL; + } + if (fold) { + kwargs = PyDict_New(); + if (kwargs == NULL) { + goto Done; + } + PyObject *obj = PyLong_FromLong(fold); + if (obj == NULL) { + goto Done; + } + int res = PyDict_SetItemString(kwargs, "fold", obj); + Py_DECREF(obj); + if (res < 0) { + goto Done; + } + } + res = PyObject_Call(cls, args, kwargs); +Done: + Py_DECREF(args); + Py_XDECREF(kwargs); + return res; +} + static PyObject * new_datetime_subclass_fold_ex(int year, int month, int day, int hour, int minute, int second, int usecond, PyObject *tzinfo, @@ -1056,15 +1089,8 @@ new_datetime_subclass_fold_ex(int year, int month, int day, int hour, int minute tzinfo, fold); } else { // Subclass - dt = PyObject_CallFunction(cls, "iiiiiiiO", - year, - month, - day, - hour, - minute, - second, - usecond, - tzinfo); + dt = call_subclass_fold(cls, fold, "iiiiiiiO", year, month, day, + hour, minute, second, usecond, tzinfo); } return dt; @@ -1120,6 +1146,23 @@ new_time_ex(int hour, int minute, int second, int usecond, #define new_time(hh, mm, ss, us, tzinfo, fold) \ new_time_ex2(hh, mm, ss, us, tzinfo, fold, &PyDateTime_TimeType) +static PyObject * +new_time_subclass_fold_ex(int hour, int minute, int second, int usecond, + PyObject *tzinfo, int fold, PyObject *cls) +{ + PyObject *t; + if ((PyTypeObject*)cls == &PyDateTime_TimeType) { + // Use the fast path constructor + t = new_time(hour, minute, second, usecond, tzinfo, fold); + } else { + // Subclass + t = call_subclass_fold(cls, fold, "iiiiO", hour, minute, second, + usecond, tzinfo); + } + + return t; +} + /* Create a timedelta instance. Normalize the members iff normalize is * true. Passing false is a speed optimization, if you know for sure * that seconds and microseconds are already in their proper ranges. In any @@ -3482,7 +3525,7 @@ datetime_date_replace_impl(PyDateTime_Date *self, int year, int month, int day) /*[clinic end generated code: output=2a9430d1e6318aeb input=0d1f02685b3e90f6]*/ { - return new_date_ex(year, month, day, Py_TYPE(self)); + return new_date_subclass_ex(year, month, day, (PyObject *)Py_TYPE(self)); } static Py_hash_t @@ -4591,8 +4634,8 @@ datetime_time_replace_impl(PyDateTime_Time *self, int hour, int minute, int fold) /*[clinic end generated code: output=0b89a44c299e4f80 input=9b6a35b1e704b0ca]*/ { - return new_time_ex2(hour, minute, second, microsecond, tzinfo, fold, - Py_TYPE(self)); + return new_time_subclass_fold_ex(hour, minute, second, microsecond, tzinfo, + fold, (PyObject *)Py_TYPE(self)); } static PyObject * @@ -6055,8 +6098,9 @@ datetime_datetime_replace_impl(PyDateTime_DateTime *self, int year, int fold) /*[clinic end generated code: output=00bc96536833fddb input=9b38253d56d9bcad]*/ { - return new_datetime_ex2(year, month, day, hour, minute, second, - microsecond, tzinfo, fold, Py_TYPE(self)); + return new_datetime_subclass_fold_ex(year, month, day, hour, minute, + second, microsecond, tzinfo, fold, + (PyObject *)Py_TYPE(self)); } static PyObject * From 6dfba7f2433986be93dca92e8391eea5a7cb7f94 Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Tue, 30 Jan 2024 22:04:54 -0500 Subject: [PATCH 2/2] Address comments --- Modules/_datetimemodule.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a9f1a8d4fe5bcc..1f53a7058a31b0 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -1046,7 +1046,8 @@ new_datetime_ex(int year, int month, int day, int hour, int minute, &PyDateTime_DateTimeType) static PyObject * -call_subclass_fold(PyObject *cls, int fold, const char *format, ...) { +call_subclass_fold(PyObject *cls, int fold, const char *format, ...) +{ PyObject *kwargs = NULL, *res = NULL; va_list va; @@ -1065,9 +1066,9 @@ call_subclass_fold(PyObject *cls, int fold, const char *format, ...) { if (obj == NULL) { goto Done; } - int res = PyDict_SetItemString(kwargs, "fold", obj); + int err = PyDict_SetItemString(kwargs, "fold", obj); Py_DECREF(obj); - if (res < 0) { + if (err < 0) { goto Done; } } @@ -1087,7 +1088,8 @@ new_datetime_subclass_fold_ex(int year, int month, int day, int hour, int minute // Use the fast path constructor dt = new_datetime(year, month, day, hour, minute, second, usecond, tzinfo, fold); - } else { + } + else { // Subclass dt = call_subclass_fold(cls, fold, "iiiiiiiO", year, month, day, hour, minute, second, usecond, tzinfo); @@ -1154,7 +1156,8 @@ new_time_subclass_fold_ex(int hour, int minute, int second, int usecond, if ((PyTypeObject*)cls == &PyDateTime_TimeType) { // Use the fast path constructor t = new_time(hour, minute, second, usecond, tzinfo, fold); - } else { + } + else { // Subclass t = call_subclass_fold(cls, fold, "iiiiO", hour, minute, second, usecond, tzinfo);