DEP: Ensure indexing errors will be raised even on empty results

Previously, when the indexing result was empty, no check was done
for backward compatibility with pre 1.9 (assumingly).
This may have been only necessary when the outer iteration is empty
as opposed to when just the inner iteration is empty, though.

In any case, it is arguably buggy to ignore indexing errors in this
case. Since there may have been a reason back in the day, and this
is probably extremely rare, optiming for a brief deprecation period
for now.

Closes gh-15898

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Matti Picus <matti.picus@gmail.com>

3 files changed, 89 insertions, 9 deletions

diff --git a/doc/release/upcoming_changes/15900.deprecation.rst b/doc/release/upcoming_changes/15900.deprecation.rst
new file mode 100644
index 000000000..22be711d0
--- /dev/null
+++ b/doc/release/upcoming_changes/15900.deprecation.rst
@@ -0,0 +1,16 @@
+Indexing errors will be reported even when index result is empty
+----------------------------------------------------------------
+In the future, NumPy will raise an IndexError when an
+integer array index contains out of bound values even if a non-indexed
+dimension is of length 0. This will now emit a DeprecationWarning.
+This can happen when the array is previously empty, or an empty
+slice is involved::
+
+    arr1 = np.zeros((5, 0))
+    arr1[[20]]
+    arr2 = np.zeros((5, 5))
+    arr2[[20], :0]
+
+Previously the non-empty index ``[20]`` was not checked for correctness.
+It will now be checked causing a deprecation warning which will be turned
+into an error. This also applies to assignments.
diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c
index 43dbde2f1..7aefbfc38 100644
--- a/numpy/core/src/multiarray/mapping.c
+++ b/numpy/core/src/multiarray/mapping.c
@@ -1655,11 +1655,12 @@ array_subscript(PyArrayObject *self, PyObject *op)
         goto finish;
     }
 
-    if (mit->numiter > 1) {
+    if (mit->numiter > 1 || mit->size == 0) {
         /*
          * If it is one, the inner loop checks indices, otherwise
          * check indices beforehand, because it is much faster if
-         * broadcasting occurs and most likely no big overhead
+         * broadcasting occurs and most likely no big overhead.
+         * The inner loop optimization skips index checks for size == 0 though.
          */
         if (PyArray_MapIterCheckIndices(mit) < 0) {
             goto finish;
@@ -2479,13 +2480,19 @@ PyArray_MapIterCheckIndices(PyArrayMapIterObject *mit)
     int i;
     NPY_BEGIN_THREADS_DEF;
 
-    if (mit->size == 0) {
-        /* All indices got broadcast away, do *not* check as it always was */
+    intp_type = PyArray_DescrFromType(NPY_INTP);
+
+    if (NpyIter_GetIterSize(mit->outer) == 0) {
+        /*
+         * When the outer iteration is empty, the indices broadcast to an
+         * empty shape, and in this case we do not check if there are out
+         * of bounds indices.
+         * The code below does use the indices without broadcasting since
+         * broadcasting only repeats values.
+         */
         return 0;
     }
 
-    intp_type = PyArray_DescrFromType(NPY_INTP);
-
     NPY_BEGIN_THREADS;
 
     for (i=0; i < mit->numiter; i++) {
@@ -2515,7 +2522,7 @@ PyArray_MapIterCheckIndices(PyArrayMapIterObject *mit)
                 if (check_and_adjust_index(&indval,
                                            outer_dim, outer_axis, _save) < 0) {
                     Py_DECREF(intp_type);
-                    return -1;
+                    goto indexing_error;
                 }
                 data += stride;
             }
@@ -2528,13 +2535,17 @@ PyArray_MapIterCheckIndices(PyArrayMapIterObject *mit)
         op_iter = NpyIter_New(op,
                         NPY_ITER_BUFFERED | NPY_ITER_NBO | NPY_ITER_ALIGNED |
                         NPY_ITER_EXTERNAL_LOOP | NPY_ITER_GROWINNER |
-                        NPY_ITER_READONLY,
+                        NPY_ITER_READONLY | NPY_ITER_ZEROSIZE_OK,
                         NPY_KEEPORDER, NPY_SAME_KIND_CASTING, intp_type);
 
         if (op_iter == NULL) {
             Py_DECREF(intp_type);
             return -1;
         }
+        if (NpyIter_GetIterSize(op_iter) == 0) {
+            NpyIter_Deallocate(op_iter);
+            continue;
+        }
 
         op_iternext = NpyIter_GetIterNext(op_iter, NULL);
         if (op_iternext == NULL) {
@@ -2554,7 +2565,7 @@ PyArray_MapIterCheckIndices(PyArrayMapIterObject *mit)
                                            outer_dim, outer_axis, _save) < 0) {
                     Py_DECREF(intp_type);
                     NpyIter_Deallocate(op_iter);
-                    return -1;
+                    goto indexing_error;
                 }
                 *iterptr += *iterstride;
             }
@@ -2567,6 +2578,32 @@ PyArray_MapIterCheckIndices(PyArrayMapIterObject *mit)
     NPY_END_THREADS;
     Py_DECREF(intp_type);
     return 0;
+
+indexing_error:
+
+    if (mit->size == 0) {
+        PyObject *err_type = NULL, *err_value = NULL, *err_traceback = NULL;
+        PyErr_Fetch(&err_type, &err_value, &err_traceback);
+        /* 2020-05-27, NumPy 1.20 */
+        if (DEPRECATE(
+                "Out of bound index found. This was previously ignored "
+                "when the indexing result contained no elements. "
+                "In the future the index error will be raised. This error "
+                "occurs either due to an empty slice, or if an array has zero "
+                "elements even before indexing.\n"
+                "(Use `warnings.simplefilter('error')` to turn this "
+                "DeprecationWarning into an error and get more details on "
+                "the invalid index.)") < 0) {
+            npy_PyErr_ChainExceptions(err_type, err_value, err_traceback);
+            return -1;
+        }
+        Py_DECREF(err_type);
+        Py_DECREF(err_value);
+        Py_XDECREF(err_traceback);
+        return 0;
+    }
+
+    return -1;
 }
 
 
diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py
index 82d24e0f7..523638a35 100644
--- a/numpy/core/tests/test_deprecations.py
+++ b/numpy/core/tests/test_deprecations.py
@@ -618,3 +618,30 @@ class BuiltInRoundComplexDType(_DeprecationTestCase):
             self.assert_not_deprecated(round, args=(scalar,))
             self.assert_not_deprecated(round, args=(scalar, 0))
             self.assert_not_deprecated(round, args=(scalar,), kwargs={'ndigits': 0})
+
+
+class TestIncorrectAdvancedIndexWithEmptyResult(_DeprecationTestCase):
+    # 2020-05-27, NumPy 1.20.0
+    message = "Out of bound index found. This was previously ignored.*"
+
+    @pytest.mark.parametrize("index", [([3, 0],), ([0, 0], [3, 0])])
+    def test_empty_subspace(self, index):
+        # Test for both a single and two/multiple advanced indices. These
+        # This will raise an IndexError in the future.
+        arr = np.ones((2, 2, 0))
+        self.assert_deprecated(arr.__getitem__, args=(index,))
+        self.assert_deprecated(arr.__setitem__, args=(index, 0.))
+
+        # for this array, the subspace is only empty after applying the slice
+        arr2 = np.ones((2, 2, 1))
+        index2 = (slice(0, 0),) + index
+        self.assert_deprecated(arr2.__getitem__, args=(index2,))
+        self.assert_deprecated(arr2.__setitem__, args=(index2, 0.))
+
+    def test_empty_index_broadcast_not_deprecated(self):
+        arr = np.ones((2, 2, 2))
+
+        index = ([[3], [2]], [])  # broadcast to an empty result.
+        self.assert_not_deprecated(arr.__getitem__, args=(index,))
+        self.assert_not_deprecated(arr.__setitem__,
+                                   args=(index, np.empty((2, 0, 2))))