-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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.
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
Add Werror to CI #56277
Add Werror to CI #56277
Changes from all commits
33f1610
62513b1
b477c32
acce2f0
5aa4640
45c73ca
851ddee
9cb8387
a5952e8
f7f8fae
41db0a7
5f13b15
c31a4aa
04739fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1236,11 +1236,11 @@ static char **NpyArr_encodeLabels(PyArrayObject *labels, PyObjectEncoder *enc, | |
} | ||
|
||
int is_datetimelike = 0; | ||
npy_int64 i8date; | ||
int64_t i8date; | ||
NPY_DATETIMEUNIT dateUnit = NPY_FR_ns; | ||
if (PyTypeNum_ISDATETIME(type_num)) { | ||
is_datetimelike = 1; | ||
i8date = *(npy_int64 *)dataptr; | ||
i8date = *(int64_t *)dataptr; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a safe cast, but leaving to another PR to fix. The proper way to do this would be via memcpy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is likely pedantic but I'm fairly certain this is a strict aliasing violation. Where/when that matters may be up for debate. The memcpy from that comment would seemingly be safer |
||
dateUnit = get_datetime_metadata_from_dtype(dtype).base; | ||
} else if (PyDate_Check(item) || PyDelta_Check(item)) { | ||
is_datetimelike = 1; | ||
|
@@ -1250,7 +1250,11 @@ static char **NpyArr_encodeLabels(PyArrayObject *labels, PyObjectEncoder *enc, | |
i8date = get_long_attr(item, "_value"); | ||
} else { | ||
if (PyDelta_Check(item)) { | ||
i8date = total_seconds(item) * 1000000000LL; // nanoseconds per second | ||
// TODO(anyone): cast below loses precision if total_seconds return | ||
// value exceeds number of bits that significand can hold | ||
// also liable to overflow | ||
i8date = (int64_t)(total_seconds(item) * | ||
1000000000LL); // nanoseconds per second | ||
} else { | ||
// datetime.* objects don't follow above rules | ||
i8date = PyDateTimeToEpoch(item, NPY_FR_ns); | ||
|
@@ -1291,7 +1295,7 @@ static char **NpyArr_encodeLabels(PyArrayObject *labels, PyObjectEncoder *enc, | |
ret = 0; | ||
break; | ||
} | ||
snprintf(cLabel, size_of_cLabel, "%" NPY_DATETIME_FMT, i8date); | ||
snprintf(cLabel, size_of_cLabel, "%" PRId64, i8date); | ||
len = strlen(cLabel); | ||
} | ||
} | ||
|
@@ -1495,10 +1499,14 @@ static void Object_beginTypeContext(JSOBJ _obj, JSONTypeContext *tc) { | |
} | ||
return; | ||
} else if (PyDelta_Check(obj)) { | ||
npy_int64 value = | ||
PyObject_HasAttrString(obj, "_value") ? get_long_attr(obj, "_value") | ||
: // pd.Timedelta object or pd.NaT | ||
total_seconds(obj) * 1000000000LL; // nanoseconds per sec | ||
// pd.Timedelta object or pd.NaT should evaluate true here | ||
// fallback to nanoseconds per sec for other objects | ||
// TODO(anyone): cast below loses precision if total_seconds return | ||
// value exceeds number of bits that significand can hold | ||
// also liable to overflow | ||
int64_t value = PyObject_HasAttrString(obj, "_value") | ||
? get_long_attr(obj, "_value") | ||
: (int64_t)(total_seconds(obj) * 1000000000LL); | ||
|
||
if (value == get_nat()) { | ||
tc->type = JT_NULL; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@phofl pointed out on macOS that npy_int64 is a typedef for a long whereas int64_t is a typedef for a long long. Unfortunately referencing these via pointer violates the strict aliasing rule
For now just picked int64_t as it was bigger. I'm not sure in practice that this matters to much, but we do freely mix these uses today