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.
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
bpo-31465: allow _PyType_Lookup() to raise exceptions #3616
base: main
Are you sure you want to change the base?
Conversation
…hained name lookups in _PyType_Lookup().
…m update_one_slot().
…r ceases to be a valid base type, there'll probably be larger code sections to change than this one.
…common in Python 3 now. Also make the step from "return NULL" error handling to "goto error" reference cleanup explicit.
Distinguish between "error" and "error with exception" cases in _PyType_LookupUncached(). Fix a reference leak of "mro" on lookup errors. Resolves issues found by Serhiy Storchaka.
…isting) API names. Suggested by Serhiy Storchaka.
…l PyObject_GetItem() for them.
…e-calculation if the mapping is not exactly a dict.
…ential re-calculation if the mapping is not exactly a dict." This reverts commit 1d52082.
… and call PyObject_GetItem() for them." This reverts commit 09e716a.
…g lookup exceptions, just like "_PyType_Lookup()" did previously.
…ce eating lookup exceptions, just like "_PyType_Lookup()" did previously." This reverts commit 4efde8e.
…) from functions that swallow live exceptions.
… handle them in the code that calls these functions. The one non-exception error case during lookup ("no MRO") is not handled since it is unlikely and not user visible. Note that this changes the signature of both functions, but they are clearly not public functions. Currently leads to crashes due to NULL returns without exceptions set. Needs investigation.
Two minor points. |
Sounds good to me. Will change it when I get to it, unless it proves controversial in the meantime.
|
This PR is stale because it has been open for 30 days with no activity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many test fails:
`Test suite interrupted by signal SIGINT.
4 tests omitted:
test_multiprocessing_fork test_multiprocessing_forkserver
test_multiprocessing_spawn test_poplib
376 tests OK.
16 tests failed:
test_asyncio test_dbm test_float test_ftplib test_httplib
test_idle test_imaplib test_logging test_nntplib test_ossaudiodev
test_shutil test_ssl test_tk test_ttk_guionly
test_urllib2_localnet test_xmlrpc_net
9 tests skipped:
test_devpoll test_kqueue test_msilib test_nis test_startfile
test_winconsoleio test_winreg test_winsound test_zipfile64
Total duration: 13 min 49 sec
Tests result: FAILURE`
cpython on _pt_lookup_exc [$?] via
Also, in typeobject.c line 739 shows:
/* FIXME: unclear if we must update the slots even on errors or not */
Need to have this cleared up before accepting PR. Get consensus from core dev(s).
This PR is stale because it has been open for 30 days with no activity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has merge conflicts now.
When you're done making the requested changes, leave the comment: And if you don't make the requested changes, you will be put in the comfy chair! |
This PR is stale because it has been open for 30 days with no activity. |
This is based off #3279, the first commits are the same, because they touch the same code. FIrst new commit is 6c61028, right after the merge of "master". 6c61028 is a related bug fix, an unchecked error case.
Probably needs new tests that exercise the error conditions.
https://bugs.python.org/issue31465