bpo-33237: Improve AttributeError message for partially initialized module. (GH-6398)

diff --git a/Include/moduleobject.h b/Include/moduleobject.h
index 1d8fe46dea03a1713e6a5424d331004f82543e79..4d173808738cc9b4600c065b7822fb3af4bc4523 100644 (file)
--- a/Include/moduleobject.h
+++ b/Include/moduleobject.h
@@ -30,6 +30,7 @@ PyAPI_FUNC(PyObject *) PyModule_GetFilenameObject(PyObject *);
 #ifndef Py_LIMITED_API
 PyAPI_FUNC(void) _PyModule_Clear(PyObject *);
 PyAPI_FUNC(void) _PyModule_ClearDict(PyObject *);
+PyAPI_FUNC(int) _PyModuleSpec_IsInitializing(PyObject *);
 #endif
 PyAPI_FUNC(struct PyModuleDef*) PyModule_GetDef(PyObject*);
 PyAPI_FUNC(void*) PyModule_GetState(PyObject*);

diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index fb9453ad0b39203b25fbb35868a8ee955c74c783..7306e0f7f722dd85ee206450d32429cfdbfe8aef 100644 (file)
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -1271,6 +1271,19 @@ class CircularImportTests(unittest.TestCase):
         except ImportError:
             self.fail('circular import with binding a submodule to a name failed')
 
+    def test_crossreference1(self):
+        import test.test_import.data.circular_imports.use
+        import test.test_import.data.circular_imports.source
+
+    def test_crossreference2(self):
+        with self.assertRaises(AttributeError) as cm:
+            import test.test_import.data.circular_imports.source
+        errmsg = str(cm.exception)
+        self.assertIn('test.test_import.data.circular_imports.source', errmsg)
+        self.assertIn('spam', errmsg)
+        self.assertIn('partially initialized module', errmsg)
+        self.assertIn('circular import', errmsg)
+
 
 if __name__ == '__main__':
     # Test needs to be a package, so we can do relative imports.

diff --git a/Lib/test/test_import/data/circular_imports/source.py b/Lib/test/test_import/data/circular_imports/source.py
new file mode 100644 (file)
index 0000000..f104099
--- /dev/null
+++ b/Lib/test/test_import/data/circular_imports/source.py
@@ -0,0 +1,2 @@
+from . import use
+spam = 1

diff --git a/Lib/test/test_import/data/circular_imports/use.py b/Lib/test/test_import/data/circular_imports/use.py
new file mode 100644 (file)
index 0000000..418f9e2
--- /dev/null
+++ b/Lib/test/test_import/data/circular_imports/use.py
@@ -0,0 +1,2 @@
+from . import source
+source.spam

diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst b/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst
new file mode 100644 (file)
index 0000000..04bd86c
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2018-07-24-12-54-57.bpo-33237.O95mps.rst
@@ -0,0 +1 @@
+Improved :exc:`AttributeError` message for partially initialized module.

diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
index ccf5f8e6d15afb29baf200030b27a025285e2371..5181941ee7bcfa8a8f9b9f578d9ed44a9d902272 100644 (file)
--- a/Objects/moduleobject.c
+++ b/Objects/moduleobject.c
@@ -698,6 +698,27 @@ module_repr(PyModuleObject *m)
     return PyObject_CallMethod(interp->importlib, "_module_repr", "O", m);
 }
 
+/* Check if the "_initializing" attribute of the module spec is set to true.
+   Clear the exception and return 0 if spec is NULL.
+ */
+int
+_PyModuleSpec_IsInitializing(PyObject *spec)
+{
+    if (spec != NULL) {
+        _Py_IDENTIFIER(_initializing);
+        PyObject *value = _PyObject_GetAttrId(spec, &PyId__initializing);
+        if (value != NULL) {
+            int initializing = PyObject_IsTrue(value);
+            Py_DECREF(value);
+            if (initializing >= 0) {
+                return initializing;
+            }
+        }
+    }
+    PyErr_Clear();
+    return 0;
+}
+
 static PyObject*
 module_getattro(PyModuleObject *m, PyObject *name)
 {
@@ -717,8 +738,24 @@ module_getattro(PyModuleObject *m, PyObject *name)
         _Py_IDENTIFIER(__name__);
         mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__);
         if (mod_name && PyUnicode_Check(mod_name)) {
-            PyErr_Format(PyExc_AttributeError,
-                        "module '%U' has no attribute '%U'", mod_name, name);
+            _Py_IDENTIFIER(__spec__);
+            Py_INCREF(mod_name);
+            PyObject *spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__);
+            Py_XINCREF(spec);
+            if (_PyModuleSpec_IsInitializing(spec)) {
+                PyErr_Format(PyExc_AttributeError,
+                             "partially initialized "
+                             "module '%U' has no attribute '%U' "
+                             "(most likely due to a circular import)",
+                             mod_name, name);
+            }
+            else {
+                PyErr_Format(PyExc_AttributeError,
+                             "module '%U' has no attribute '%U'",
+                             mod_name, name);
+            }
+            Py_XDECREF(spec);
+            Py_DECREF(mod_name);
             return NULL;
         }
     }

diff --git a/Python/import.c b/Python/import.c
index 1e8c07b0b245cc515f320813b107399d69efad9e..e761f65c66ba7f0ccd358230d0a7e924d822f4b9 100644 (file)
--- a/Python/import.c
+++ b/Python/import.c
@@ -1721,11 +1721,8 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
     mod = PyImport_GetModule(abs_name);
     if (mod != NULL && mod != Py_None) {
         _Py_IDENTIFIER(__spec__);
-        _Py_IDENTIFIER(_initializing);
         _Py_IDENTIFIER(_lock_unlock_module);
-        PyObject *value = NULL;
         PyObject *spec;
-        int initializing = 0;
 
         /* Optimization: only call _bootstrap._lock_unlock_module() if
            __spec__._initializing is true.
@@ -1733,26 +1730,17 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals,
            stuffing the new module in sys.modules.
          */
         spec = _PyObject_GetAttrId(mod, &PyId___spec__);
-        if (spec != NULL) {
-            value = _PyObject_GetAttrId(spec, &PyId__initializing);
-            Py_DECREF(spec);
-        }
-        if (value == NULL)
-            PyErr_Clear();
-        else {
-            initializing = PyObject_IsTrue(value);
-            Py_DECREF(value);
-            if (initializing == -1)
-                PyErr_Clear();
-            if (initializing > 0) {
-                value = _PyObject_CallMethodIdObjArgs(interp->importlib,
-                                                &PyId__lock_unlock_module, abs_name,
-                                                NULL);
-                if (value == NULL)
-                    goto error;
-                Py_DECREF(value);
+        if (_PyModuleSpec_IsInitializing(spec)) {
+            PyObject *value = _PyObject_CallMethodIdObjArgs(interp->importlib,
+                                            &PyId__lock_unlock_module, abs_name,
+                                            NULL);
+            if (value == NULL) {
+                Py_DECREF(spec);
+                goto error;
             }
+            Py_DECREF(value);
         }
+        Py_XDECREF(spec);
     }
     else {
         Py_XDECREF(mod);