diff --git a/Include/moduleobject.h b/Include/moduleobject.h index 1d8fe46dea0..4d173808738 100644 --- 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 fb9453ad0b3..7306e0f7f72 100644 --- 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 index 00000000000..f104099048c --- /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 index 00000000000..418f9e2688e --- /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 index 00000000000..04bd86c3dd6 --- /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 ccf5f8e6d15..5181941ee7b 100644 --- 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 1e8c07b0b24..e761f65c66b 100644 --- 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);