10000 general code cleanup by gandalf013 · Pull Request #184 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

general code cleanup #184

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 2 commits into from
Closed

general code cleanup #184

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e141ff1
general code cleanup
gandalf013 Jan 16, 2012
ad78432
indentation, add semicolons in some places where they were not being …
gandalf013 Jan 21, 2012
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
Next Next commit
general code cleanup
  • Loading branch information
@gandalf013
gandalf013 committed Jan 16, 2012
commit e141ff1e48eaac88f49963420d3c28df28bab111
60 changes: 30 additions & 30 deletions numpy/core/include/numpy/ndarraytypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,21 +956,21 @@ typedef int (PyArray_FinalizeFunc)(PyArrayObject *, PyObject *);
#if NPY_ALLOW_THREADS
#define NPY_BEGIN_ALLOW_THREADS Py_BEGIN_ALLOW_THREADS
#define NPY_END_ALLOW_THREADS Py_END_ALLOW_THREADS
#define NPY_BEGIN_THREADS_DEF PyThreadState *_save=NULL;
#define NPY_BEGIN_THREADS _save = PyEval_SaveThread();
#define NPY_END_THREADS do {if (_save) PyEval_RestoreThread(_save);} while (0);
#define NPY_BEGIN_THREADS_DEF PyThreadState *_save=NULL
Copy link
Member

Choose a reason for hiding this comment

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

A potential problem with this change is that originally most of these macros were used without a trailing semicolon. I tried to clean most of those uses up, but some may remain. Have you tested this or audited the places they are used? I'm also concerned about third party code, although they should be using semicolons, but they may have just copied earlier usage.
Since double semicolons don't cause compiler problems, so I'd just as soon leave them in even if it is a bit ugly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am mixed on this. The code uses NPY_BEGIN_THREADS*, NPY_END_THREADS, and the non-NPY_ prefix versions of the macros with semicolons everywhere, so getting rid of the semicolons there is safe. Of course, if there is third-party code out there that doesn't use the semicolons, then it will break. I have no idea if this is the case or not.

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 in general it's better forcing the usage to have the semicolon, some auto-indent code behaves poorly with that. I agree with Chuck about third party code - this doesn't affect the ABI, but some third party code might have to add a semicolon. If they do this, their code will still work with older NumPy versions, so it's probably fine.

#define NPY_BEGIN_THREADS do {_save = PyEval_SaveThread();} while (0)
#define NPY_END_THREADS do {if (_save) PyEval_RestoreThread(_save);} while (0)
Copy link
Member

Choose a reason for hiding this comment

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

That's definitely safer with the if statement in there. I'm not sure the previous one is needed but it probably doesn't hurt.


#define NPY_BEGIN_THREADS_DESCR(dtype) \
do {if (!(PyDataType_FLAGCHK(dtype, NPY_NEEDS_PYAPI))) \
NPY_BEGIN_THREADS;} while (0);
NPY_BEGIN_THREADS;} while (0)

#define NPY_END_THREADS_DESCR(dtype) \
do {if (!(PyDataType_FLAGCHK(dtype, NPY_NEEDS_PYAPI))) \
NPY_END_THREADS; } while (0);
NPY_END_THREADS; } while (0)

#define NPY_ALLOW_C_API_DEF PyGILState_STATE __save__;
#define NPY_ALLOW_C_API __save__ = PyGILState_Ensure();
#define NPY_DISABLE_C_API PyGILState_Release(__save__);
#define NPY_ALLOW_C_API_DEF PyGILState_STATE __save__
#define NPY_ALLOW_C_API do {__save__ = PyGILState_Ensure();} while (0)
#define NPY_DISABLE_C_API do {PyGILState_Release(__save__);} while (0)
Copy link
Member

Choose a reason for hiding this comment

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

The last two probably aren't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They aren't needed in the current code. I am just safeguarding for something like

if (condition)
    NPY_ALLOW_C_API;
else ()...

But this probably will never happen.

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest it's needed, because these macros are exposed externally, and third party code might want to do what gandalf's example shows.

#else
#define NPY_BEGIN_ALLOW_THREADS
#define NPY_END_ALLOW_THREADS
Expand Down Expand Up @@ -1112,19 +1112,19 @@ struct PyArrayIterObject_tag {
#define PyArrayIter_Check(op) PyObject_TypeCheck(op, &PyArrayIter_Type)

#define _PyAIT(it) ((PyArrayIterObject *)(it))
#define PyArray_ITER_RESET(it) { \
#define PyArray_ITER_RESET(it) do { \
_PyAIT(it)->index = 0; \
_PyAIT(it)->dataptr = PyArray_BYTES(_PyAIT(it)->ao); \
memset(_PyAIT(it)->coordinates, 0, \
(_PyAIT(it)->nd_m1+1)*sizeof(npy_intp)); \
}
} while (0)

#define _PyArray_ITER_NEXT1(it) { \
#define _PyArray_ITER_NEXT1(it) do { \
(it)->dataptr += _PyAIT(it)->strides[0]; \
(it)->coordinates[0]++; \
}
} while (0)

#define _PyArray_ITER_NEXT2(it) { \
#define _PyArray_ITER_NEXT2(it) do { \
if ((it)->coordinates[1] < (it)->dims_m1[1]) { \
(it)->coordinates[1]++; \
(it)->dataptr += (it)->strides[1]; \
Expand All @@ -1135,9 +1135,9 @@ struct PyArrayIterObject_tag {
(it)->dataptr += (it)->strides[0] - \
(it)->backstrides[1]; \
} \
}
} while (0)

#define _PyArray_ITER_NEXT3(it) { \
#define _PyArray_ITER_NEXT3(it) do { \
if ((it)->coordinates[2] < (it)->dims_m1[2]) { \
(it)->coordinates[2]++; \
(it)->dataptr += (it)->strides[2]; \
Expand All @@ -1156,9 +1156,9 @@ struct PyArrayIterObject_tag {
(it)->backstrides[1]; \
} \
} \
}
} while (0)

#define PyArray_ITER_NEXT(it) { \
#define PyArray_ITER_NEXT(it) do { \
_PyAIT(it)->index++; \
if (_PyAIT(it)->nd_m1 == 0) { \
_PyArray_ITER_NEXT1(_PyAIT(it)); \
Expand All @@ -1185,9 +1185,9 @@ struct PyArrayIterObject_tag {
} \
} \
} \
}
} while (0)

#define PyArray_ITER_GOTO(it, destination) { \
#define PyArray_ITER_GOTO(it, destination) do { \
int __npy_i; \
_PyAIT(it)->index = 0; \
_PyAIT(it)->dataptr = PyArray_BYTES(_PyAIT(it)->ao); \
Expand All @@ -1204,9 +1204,9 @@ struct PyArrayIterObject_tag {
( __npy_i==_PyAIT(it)->nd_m1 ? 1 : \
_PyAIT(it)->dims_m1[__npy_i+1]+1) ; \
} \
}
} while (0)

#define PyArray_ITER_GOTO1D(it, ind) { \
#define PyArray_ITER_GOTO1D(it, ind) do { \
int __npy_i; \
npy_intp __npy_ind = (npy_intp) (ind); \
if (__npy_ind < 0) __npy_ind += _PyAIT(it)->size; \
Expand All @@ -1228,7 +1228,7 @@ struct PyArrayIterObject_tag {
__npy_ind %= _PyAIT(it)->factors[__npy_i]; \
} \
} \
}
} while (0)

#define PyArray_ITER_DATA(it) ((void *)(_PyAIT(it)->dataptr))

Expand All @@ -1251,37 +1251,37 @@ typedef struct {
} PyArrayMultiIterObject;

#define _PyMIT(m) ((PyArrayMultiIterObject *)(m))
#define PyArray_MultiIter_RESET(multi) { \
#define PyArray_MultiIter_RESET(multi) do { \
int __npy_mi; \
_PyMIT(multi)->index = 0; \
for (__npy_mi=0; __npy_mi < _PyMIT(multi)->numiter; __npy_mi++) { \
PyArray_ITER_RESET(_PyMIT(multi)->iters[__npy_mi]); \
} \
}
} while (0)

#define PyArray_MultiIter_NEXT(multi) { \
#define PyArray_MultiIter_NEXT(multi) do { \
int __npy_mi; \
_PyMIT(multi)->index++; \
for (__npy_mi=0; __npy_mi < _PyMIT(multi)->numiter; __npy_mi++) { \
PyArray_ITER_NEXT(_PyMIT(multi)->iters[__npy_mi]); \
} \
}
} while (0)

#define PyArray_MultiIter_GOTO(multi, dest) { \
#define PyArray_MultiIter_GOTO(multi, dest) do { \
int __npy_mi; \
for (__npy_mi=0; __npy_mi < _PyMIT(multi)->numiter; __npy_mi++) { \
PyArray_ITER_GOTO(_PyMIT(multi)->iters[__npy_mi], dest); \
} \
_PyMIT(multi)->index = _PyMIT(multi)->iters[0]->index; \
}
} while (0)

#define PyArray_MultiIter_GOTO1D(multi, ind) { \
#define PyArray_MultiIter_GOTO1D(multi, ind) do { \
int __npy_mi; \
for (__npy_mi=0; __npy_mi < _PyMIT(multi)->numiter; __npy_mi++) { \
PyArray_ITER_GOTO1D(_PyMIT(multi)->iters[__npy_mi], ind); \
} \
_PyMIT(multi)->index = _PyMIT(multi)->iters[0]->index; \
}
} while (0)

#define PyArray_MultiIter_DATA(multi, i) \
((void *)(_PyMIT(multi)->iters[i]->dataptr))
Expand Down
4 changes: 2 additions & 2 deletions numpy/core/src/multiarray/iterators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,11 @@ PyArray_BroadcastToShape(PyObject *obj, npy_intp *dims, int nd)
goto err;
}
it = (PyArrayIterObject *)PyArray_malloc(sizeof(PyArrayIterObject));
PyObject_Init((PyObject *)it, &PyArrayIter_Type);

if (it == NULL) {
return NULL;
}
PyObject_Init((PyObject *)it, &PyArrayIter_Type);
Copy link
Member

Choose a reason for hiding this comment

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

OK.


PyArray_UpdateFlags(ao, NPY_ARRAY_C_CONTIGUOUS);
if (PyArray_ISCONTIGUOUS(ao)) {
it->contiguous = 1;
Expand Down
0