From c1f4db0a9b4a5e5cdb9d7bb2a52d7d2501d49a84 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Thu, 21 Dec 2023 09:55:54 +0100 Subject: [PATCH 1/2] Increase use of common get_or_try_init_type_ref helper when caching type objects. --- src/sync.rs | 4 ++-- src/types/mapping.rs | 10 +++------- src/types/sequence.rs | 10 +++------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 91a83106..62843113 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -1,5 +1,5 @@ //! Synchronization mechanisms based on the Python GIL. -use crate::{types::PyString, types::PyType, Py, PyErr, PyVisit, Python}; +use crate::{types::PyString, types::PyType, Py, PyResult, PyVisit, Python}; use std::cell::UnsafeCell; /// Value with concurrent access protected by the GIL. @@ -196,7 +196,7 @@ impl GILOnceCell> { py: Python<'py>, module_name: &str, attr_name: &str, - ) -> Result<&'py PyType, PyErr> { + ) -> PyResult<&'py PyType> { self.get_or_try_init(py, || py.import(module_name)?.getattr(attr_name)?.extract()) .map(|ty| ty.as_ref(py)) } diff --git a/src/types/mapping.rs b/src/types/mapping.rs index bf6a6d49..583fc468 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -240,14 +240,10 @@ impl<'py> PyMappingMethods<'py> for Py2<'py, PyMapping> { } } -static MAPPING_ABC: GILOnceCell> = GILOnceCell::new(); - fn get_mapping_abc(py: Python<'_>) -> PyResult<&PyType> { - MAPPING_ABC - .get_or_try_init(py, || { - py.import("collections.abc")?.getattr("Mapping")?.extract() - }) - .map(|ty| ty.as_ref(py)) + static MAPPING_ABC: GILOnceCell> = GILOnceCell::new(); + + MAPPING_ABC.get_or_try_init_type_ref(py, "collections.abc", "Mapping") } impl PyTypeCheck for PyMapping { diff --git a/src/types/sequence.rs b/src/types/sequence.rs index c4c2b21b..beded132 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -523,14 +523,10 @@ where Ok(v) } -static SEQUENCE_ABC: GILOnceCell> = GILOnceCell::new(); - fn get_sequence_abc(py: Python<'_>) -> PyResult<&PyType> { - SEQUENCE_ABC - .get_or_try_init(py, || { - py.import("collections.abc")?.getattr("Sequence")?.extract() - }) - .map(|ty| ty.as_ref(py)) + static SEQUENCE_ABC: GILOnceCell> = GILOnceCell::new(); + + SEQUENCE_ABC.get_or_try_init_type_ref(py, "collections.abc", "Sequence") } impl PyTypeCheck for PySequence { From 3c97167fd139701d300e8935cc689edc91bdb41d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Thu, 21 Dec 2023 09:59:24 +0100 Subject: [PATCH 2/2] Use write_unraisable to report errors loading type objects for ABC checks. --- src/types/mapping.rs | 6 ++++-- src/types/sequence.rs | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/types/mapping.rs b/src/types/mapping.rs index 583fc468..3199c548 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -256,8 +256,10 @@ impl PyTypeCheck for PyMapping { PyDict::is_type_of(object) || get_mapping_abc(object.py()) .and_then(|abc| object.is_instance(abc)) - // TODO: surface errors in this chain to the user - .unwrap_or(false) + .unwrap_or_else(|err| { + err.write_unraisable(object.py(), Some(object)); + false + }) } } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index beded132..cb1599d6 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -540,8 +540,10 @@ impl PyTypeCheck for PySequence { || PyTuple::is_type_of(object) || get_sequence_abc(object.py()) .and_then(|abc| object.is_instance(abc)) - // TODO: surface errors in this chain to the user - .unwrap_or(false) + .unwrap_or_else(|err| { + err.write_unraisable(object.py(), Some(object)); + false + }) } }