Make "cimport numpy" without import_array() safer by automatically calling it (GH-3524)

12 files changed, 200 insertions, 8 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index ab584fbe7..1212da977 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -234,6 +234,11 @@ Other changes
 
 * Support for Python 2.6 was removed.
 
+* ``numpy.import_array`` is automatically called if ``numpy`` has been
+  ``cimported`` and it hasn't been called manually. This is intended
+  as a hidden fail-safe so user code should continue to call
+  ``numpy.import_array``. (Github issue #3524)
+
 
 0.29.17 (2020-0?-??)
 ====================
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index d28aef342..ca417f172 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -8159,6 +8159,33 @@ utility_code_for_imports = {
     'inspect': ("__Pyx_patch_inspect", "PatchInspect", "Coroutine.c"),
 }
 
+def cimport_numpy_check(node, code):
+    # shared code between CImportStatNode and FromCImportStatNode
+    # check to ensure that import_array is called
+    for mod in code.globalstate.module_node.scope.cimported_modules:
+        if mod.name != node.module_name:
+            continue
+        # there are sometimes several cimported modules with the same name
+        # so complete the loop if necessary
+        import_array = mod.lookup_here("import_array")
+        _import_array = mod.lookup_here("_import_array")
+        # at least one entry used
+        used = (import_array and import_array.used) or (_import_array and _import_array.used)
+        if ((import_array or _import_array) # at least one entry found
+                and not used):
+            # sanity check that this is actually numpy and not a user pxd called "numpy"
+            if _import_array and _import_array.type.is_cfunction:
+                # warning is mainly for the sake of testing
+                warning(node.pos, "'numpy.import_array()' has been added automatically "
+                        "since 'numpy' was cimported but 'numpy.import_array' was not called.", 0)
+                from .Code import TempitaUtilityCode
+                code.globalstate.use_utility_code(
+                         TempitaUtilityCode.load_cached("NumpyImportArray", "NumpyImportArray.c",
+                                            context = {'err_goto': code.error_goto(node.pos)})
+                    )
+                return # no need to continue once the utility code is added
+
+
 
 class CImportStatNode(StatNode):
     #  cimport statement
@@ -8200,7 +8227,8 @@ class CImportStatNode(StatNode):
         return self
 
     def generate_execution_code(self, code):
-        pass
+        if self.module_name == "numpy":
+            cimport_numpy_check(self, code)
 
 
 class FromCImportStatNode(StatNode):
@@ -8279,7 +8307,8 @@ class FromCImportStatNode(StatNode):
         return self
 
     def generate_execution_code(self, code):
-        pass
+        if self.module_name == "numpy":
+            cimport_numpy_check(self, code)
 
 
 class FromImportStatNode(StatNode):
diff --git a/Cython/Compiler/TypeInference.py b/Cython/Compiler/TypeInference.py
index 6f89b134b..73939e155 100644
--- a/Cython/Compiler/TypeInference.py
+++ b/Cython/Compiler/TypeInference.py
@@ -178,7 +178,7 @@ class MarkParallelAssignments(EnvTransform):
         return node
 
     def visit_FromCImportStatNode(self, node):
-        pass # Can't be assigned to...
+        return node # Can't be assigned to...
 
     def visit_FromImportStatNode(self, node):
         for name, target in node.items:
diff --git a/Cython/Includes/numpy/__init__.pxd b/Cython/Includes/numpy/__init__.pxd
index 025b503ec..3e765498e 100644
--- a/Cython/Includes/numpy/__init__.pxd
+++ b/Cython/Includes/numpy/__init__.pxd
@@ -430,6 +430,9 @@ cdef extern from "numpy/arrayobject.h":
         int len
 
     int _import_array() except -1
+    # A second definition so _import_array isn't marked as used when we use it here.
+    # Do not use - subject to change any time.
+    int __pyx_import_array "_import_array"() except -1
 
     #
     # Macros from ndarrayobject.h
@@ -1046,7 +1049,7 @@ cdef inline object get_array_base(ndarray arr):
 # Cython code.
 cdef inline int import_array() except -1:
     try:
-        _import_array()
+        __pyx_import_array()
     except Exception:
         raise ImportError("numpy.core.multiarray failed to import")
 
diff --git a/Cython/Utility/NumpyImportArray.c b/Cython/Utility/NumpyImportArray.c
new file mode 100644
index 000000000..aa8a18fea
--- /dev/null
+++ b/Cython/Utility/NumpyImportArray.c
@@ -0,0 +1,21 @@
+///////////////////////// NumpyImportArray.init ////////////////////
+
+// comment below is deliberately kept in the generated C file to
+// help users debug where this came from:
+/*
+ * Cython has automatically inserted a call to _import_array since
+ * you didn't include one when you cimported numpy. To disable this
+ * add the line
+ *   <void>numpy._import_array
+ */
+#ifdef NPY_NDARRAYOBJECT_H /* numpy headers have been included */
+// NO_IMPORT_ARRAY is Numpy's mechanism for indicating that import_array is handled elsewhere
+#if !NO_IMPORT_ARRAY /* https://docs.scipy.org/doc/numpy-1.17.0/reference/c-api.array.html#c.NO_IMPORT_ARRAY  */
+if (unlikely(_import_array() == -1)) {
+    PyErr_SetString(PyExc_ImportError, "numpy.core.multiarray failed to import "
+    "(auto-generated because you didn't call 'numpy.import_array()' after cimporting numpy; "
+    "use '<void>numpy._import_array' to disable if you are certain you don't need it).");
+    {{ err_goto }};
+}
+#endif
+#endif
diff --git a/tests/errors/w_numpy_arr_as_cppvec_ref.pyx b/tests/errors/w_numpy_arr_as_cppvec_ref.pyx
index e7fbaeece..b8dd1f536 100644
--- a/tests/errors/w_numpy_arr_as_cppvec_ref.pyx
+++ b/tests/errors/w_numpy_arr_as_cppvec_ref.pyx
@@ -5,6 +5,8 @@ import numpy as np
 cimport numpy as np
 from libcpp.vector cimport vector
 
+np.import_array()
+
 cdef extern from *:
     void cpp_function_vector1(vector[int])
     void cpp_function_vector2(vector[int] &)
@@ -24,8 +26,8 @@ def main():
 
 
 _ERRORS = """
-17:25: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
-18:25: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
-19:28: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
-19:33: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
+19:25: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
+20:25: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
+21:28: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
+21:33: Cannot pass Python object as C++ data structure reference (vector[int] &), will pass by copy.
 """
diff --git a/tests/run/numpy_cimport_1.pyx b/tests/run/numpy_cimport_1.pyx
new file mode 100644
index 000000000..05754f591
--- /dev/null
+++ b/tests/run/numpy_cimport_1.pyx
@@ -0,0 +1,25 @@
+# mode: run
+# tag: warnings, numpy
+
+cimport numpy as np
+# np.import_array not called - should generate warning
+
+cdef extern from *:
+    """
+    static void** _check_array_api(void) {
+        return PyArray_API; /* should be non NULL */
+    }
+    """
+    void** _check_array_api()
+
+def check_array_api():
+    """
+    >>> check_array_api()
+    True
+    """
+    return _check_array_api() != NULL
+
+
+_WARNINGS = """
+4:8: 'numpy.import_array()' has been added automatically since 'numpy' was cimported but 'numpy.import_array' was not called.
+"""
diff --git a/tests/run/numpy_cimport_2.pyx b/tests/run/numpy_cimport_2.pyx
new file mode 100644
index 000000000..cdf8783c9
--- /dev/null
+++ b/tests/run/numpy_cimport_2.pyx
@@ -0,0 +1,25 @@
+# mode: run
+# tag: warnings, numpy
+
+cimport numpy as np
+np.import_array()
+# np.import_array is called - no warning necessary
+
+cdef extern from *:
+    """
+    static void** _check_array_api(void) {
+        return PyArray_API; /* should be non NULL */
+    }
+    """
+    void** _check_array_api()
+
+def check_array_api():
+    """
+    >>> check_array_api()
+    True
+    """
+    return _check_array_api() != NULL
+
+
+_WARNINGS = """
+"""
diff --git a/tests/run/numpy_cimport_3.pyx b/tests/run/numpy_cimport_3.pyx
new file mode 100644
index 000000000..b5b24661d
--- /dev/null
+++ b/tests/run/numpy_cimport_3.pyx
@@ -0,0 +1,8 @@
+# mode: compile
+# tag: warnings, numpy
+
+import numpy as np
+# Numpy is only imported - no warning necessary
+
+_WARNINGS = """
+"""
diff --git a/tests/run/numpy_cimport_4.pyx b/tests/run/numpy_cimport_4.pyx
new file mode 100644
index 000000000..44e112054
--- /dev/null
+++ b/tests/run/numpy_cimport_4.pyx
@@ -0,0 +1,24 @@
+# mode: run
+# tag: warnings, numpy
+
+cimport numpy
+<void>numpy.import_array # dummy call should stop Cython auto-generating call to import_array
+
+cdef extern from *:
+    """
+    static void** _check_array_api(void) {
+        return PyArray_API; /* should be non NULL if initialized */
+    }
+    """
+    void** _check_array_api()
+
+def check_array_api():
+    """
+    >>> check_array_api()
+    True
+    """
+    return _check_array_api() == NULL # not initialized
+
+
+_WARNINGS = """
+"""
diff --git a/tests/run/numpy_cimport_5.pyx b/tests/run/numpy_cimport_5.pyx
new file mode 100644
index 000000000..2768f2dae
--- /dev/null
+++ b/tests/run/numpy_cimport_5.pyx
@@ -0,0 +1,25 @@
+# mode: run
+# tag: warnings, numpy
+
+from numpy cimport ndarray
+# np.import_array not called - should generate warning
+
+cdef extern from *:
+    """
+    static void** _check_array_api(void) {
+        return PyArray_API; /* should be non NULL */
+    }
+    """
+    void** _check_array_api()
+
+def check_array_api():
+    """
+    >>> check_array_api()
+    True
+    """
+    return _check_array_api() != NULL
+
+
+_WARNINGS = """
+4:0: 'numpy.import_array()' has been added automatically since 'numpy' was cimported but 'numpy.import_array' was not called.
+"""
diff --git a/tests/run/numpy_cimport_6.pyx b/tests/run/numpy_cimport_6.pyx
new file mode 100644
index 000000000..5cda5246d
--- /dev/null
+++ b/tests/run/numpy_cimport_6.pyx
@@ -0,0 +1,25 @@
+# mode: run
+# tag: warnings, numpy
+
+from numpy cimport ndarray, import_array
+import_array()
+# np.import_array is called - no warning necessary
+
+cdef extern from *:
+    """
+    static void** _check_array_api(void) {
+        return PyArray_API; /* should be non NULL */
+    }
+    """
+    void** _check_array_api()
+
+def check_array_api():
+    """
+    >>> check_array_api()
+    True
+    """
+    return _check_array_api() != NULL
+
+
+_WARNINGS = """
+"""