-
Notifications
You must be signed in to change notification settings - Fork 1
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
fixes from compiling with warnings #17
base: graal-team/hpy
Are you sure you want to change the base?
Changes from 8 commits
9653fe5
47ab51b
37986b0
01829b6
d82d3a0
cb60cf6
ccac6a1
6c3201b
1fa8ee7
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 |
---|---|---|
|
@@ -257,7 +257,7 @@ PyArrayIdentityHash_SetItem(PyArrayIdentityHash *tb, PyObject *cache_owner, | |
|
||
|
||
NPY_NO_EXPORT PyObject * | ||
PyArrayIdentityHash_GetItem(PyObject *cache_owner, PyArrayIdentityHash const *tb, PyObject *const *key) | ||
PyArrayIdentityHash_GetItem(PyArrayIdentityHash const *tb, PyObject *cache_owner, PyObject *const *key) | ||
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 seems wrong? The argument order should be the same as 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. Okay, makes sense. Seems that I was just not consistent in adding the |
||
{ | ||
HPyContext *ctx = npy_get_context(); | ||
HPy h_cache_owner = HPy_FromPyObject(ctx, cache_owner); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,18 +108,21 @@ NPY_NO_EXPORT HPyType_Spec HPyArray_PyIntAbstractDType_spec = { | |
.name = "numpy._IntegerAbstractDType", | ||
.basicsize = sizeof(PyArray_Descr), | ||
.flags = Py_TPFLAGS_DEFAULT, | ||
.builtin_shape = HPyType_BuiltinShape_Legacy | ||
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. If there is a 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. right, but as far as I can tell there is no such use of the HEPLERS. |
||
}; | ||
|
||
NPY_NO_EXPORT HPyType_Spec HPyArray_PyFloatAbstractDType_spec = { | ||
.name = "numpy._FloatAbstractDType", | ||
.basicsize = sizeof(PyArray_Descr), | ||
.flags = HPy_TPFLAGS_DEFAULT, | ||
.builtin_shape = HPyType_BuiltinShape_Legacy | ||
}; | ||
|
||
NPY_NO_EXPORT HPyType_Spec HPyArray_PyComplexAbstractDType_spec = { | ||
.name = "numpy._ComplexAbstractDType", | ||
.basicsize = sizeof(PyArray_Descr), | ||
.flags = HPy_TPFLAGS_DEFAULT, | ||
.builtin_shape = HPyType_BuiltinShape_Legacy | ||
}; | ||
|
||
// "forward" declarations: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -224,12 +224,13 @@ HPyType_LEGACY_HELPERS(PyArrayMethodObject) | |
* on the `ArrayMethod` itself. | ||
*/ | ||
typedef struct { | ||
PyObject_HEAD | ||
HPyField *dtypes; /* PyArray_DTypeMeta **dtypes */ | ||
HPyField method; /* PyArrayMethodObject *method */ | ||
int nargs; /* method->nin + method->nout */ | ||
} PyBoundArrayMethodObject; | ||
|
||
HPyType_HELPERS(PyBoundArrayMethodObject) | ||
HPyType_LEGACY_HELPERS(PyBoundArrayMethodObject) | ||
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. I guess this had to become a legacy type so that someone could call 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. 🤔 good point. I don't think we ever specified that. 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. PyPy works around it but it would be more efficient if the HELPER was correct. |
||
|
||
|
||
extern NPY_NO_EXPORT PyTypeObject *PyArrayMethod_Type; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -630,7 +630,7 @@ static NPY_INLINE npy_longdouble | |
hpy_string_to_long_double(HPyContext *ctx, HPy op) | ||
{ | ||
const char *s; | ||
const char *end; | ||
char *end; | ||
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 caused many warnings, so either my change or all the uses of |
||
npy_longdouble temp; | ||
HPy b; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2331,7 +2331,7 @@ HPyArray_FromAny(HPyContext *ctx, HPy op, HPy newtype, int min_depth, | |
} | ||
|
||
// TODO HPY LABS PORT | ||
PyArrayObject *py_ret = HPy_AsPyObject(ctx, ret); | ||
PyArrayObject *py_ret = (PyArrayObject *)HPy_AsPyObject(ctx, ret); | ||
PyObject *py_op = HPy_AsPyObject(ctx, op); | ||
if (setArrayFromSequence(py_ret, py_op, 0, NULL) < 0) { | ||
Py_DECREF(py_ret); | ||
|
@@ -4986,7 +4986,7 @@ HPyArray_ArangeObj(HPyContext *ctx, HPy start, HPy stop, HPy step, HPy /* PyArra | |
PyObject *new; | ||
PyObject *py_range = HPy_AsPyObject(ctx, range); | ||
CAPI_WARN("calling PyArray_Byteswap"); | ||
new = PyArray_Byteswap(py_range, 1); | ||
new = PyArray_Byteswap((PyArrayObject *)py_range, 1); | ||
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. Should we perhaps do 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. I think this was has fewer casts, since most places this is used prefer a |
||
Py_DECREF(py_range); | ||
Py_DECREF(new); | ||
_hpy_set_descr(ctx, range, range_struct, dtype); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1533,7 +1533,7 @@ datetime_as_string_impl(HPyContext *ctx, HPy NPY_UNUSED(ignored), const HPy *arg | |
op_flags[1] = NPY_ITER_WRITEONLY| | ||
NPY_ITER_ALLOCATE; | ||
|
||
iter = NpyIter_MultiNew(2, op, flags, NPY_KEEPORDER, NPY_UNSAFE_CASTING, | ||
iter = HNpyIter_MultiNew(ctx, 2, op, flags, NPY_KEEPORDER, NPY_UNSAFE_CASTING, | ||
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. typo? 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. Since 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. Yes, we probably just overlooked that. |
||
op_flags, op_dtypes); | ||
if (iter == NULL) { | ||
goto fail; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1414,11 +1414,12 @@ hpy_prepare_index(HPyContext *ctx, HPy h_self, PyArrayObject *self, HPy h_index, | |
} | ||
} | ||
else if (used_ndim > PyArray_NDIM(self)) { | ||
HPyErr_SetString(ctx, ctx->h_IndexError, | ||
// HPyErr_SetString does not take extra arguments | ||
mattip marked this conversation as resolved.
Show resolved
Hide resolved
|
||
HPyErr_Format(ctx, ctx->h_IndexError, | ||
"too many indices for array: " | ||
"array is %d-dimensional, but %d were indexed" | ||
/*,PyArray_NDIM(self), | ||
used_ndim*/); | ||
"array is %d-dimensional, but %d were indexed", | ||
PyArray_NDIM(self), | ||
used_ndim); | ||
goto failed_building_indices; | ||
} | ||
else if (index_ndim == 0) { | ||
|
@@ -2641,6 +2642,7 @@ NPY_NO_EXPORT int | |
array_assign_item_slot_impl(HPyContext *ctx, HPy /* PyArrayObject * */ self, HPy_ssize_t i, HPy op) | ||
{ | ||
npy_index_info indices[2]; | ||
hpy_npy_index_info hpy_indices[2]; | ||
|
||
if (HPy_IsNull(op)) { | ||
HPyErr_SetString(ctx, ctx->h_ValueError, | ||
|
@@ -2664,6 +2666,8 @@ array_assign_item_slot_impl(HPyContext *ctx, HPy /* PyArrayObject * */ self, HPy | |
|
||
indices[0].value = i; | ||
indices[0].type = HAS_INTEGER; | ||
hpy_indices[0].value = i; | ||
hpy_indices[0].type = HAS_INTEGER; | ||
if (PyArray_NDIM(self_struct) == 1) { | ||
char *item; | ||
if (get_item_pointer(self_struct, &item, indices, 1) < 0) { | ||
|
@@ -2677,9 +2681,9 @@ array_assign_item_slot_impl(HPyContext *ctx, HPy /* PyArrayObject * */ self, HPy | |
else { | ||
HPy view; // PyArrayObject * | ||
|
||
indices[1].value = PyArray_NDIM(self_struct) - 1; | ||
indices[1].type = HAS_ELLIPSIS; | ||
if (hpy_get_view_from_index(ctx, self, self_struct, &view, indices, 2, 0) < 0) { | ||
hpy_indices[1].value = PyArray_NDIM(self_struct) - 1; | ||
hpy_indices[1].type = HAS_ELLIPSIS; | ||
if (hpy_get_view_from_index(ctx, self, self_struct, &view, hpy_indices, 2, 0) < 0) { | ||
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. I needed to duplicate the 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. Should 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. No. Only |
||
return -1; | ||
} | ||
PyArrayObject *view_struct = PyArrayObject_AsStruct(ctx, view); | ||
|
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.
Do you know from the top of your head if
PyArrayFlagsObject
still needs to be legacy? It looks like it could be a pure HPy type but we just forgot to removePyObject_HEAD
.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.
No, I don't know. We could check it in a follow-up PR