Merge #3171

3171: RFC: Simplify version handling of UI tests. r=davidhewitt a=adamreichold

 Due to checking in error outputs these tests only work reliably on stable and not on intermediate version between our MSRV (currently 1.48) and the current stable version.

Hence this simplifies things to run only MSRV-compatible tests for the MSRV builds, anything else for stable builds except for those tests which require the nightly feature, i.e. the `Ungil` being distinct from the `Send` trait.

Finally, `not_send3` is disabled when using the nightly feature since while `Rc` is not send, it also not GIL-bound and hence can be passed into `allow_threads` as it does not actually spawn a new thread.


Co-authored-by: Adam Reichold <adam.reichold@t-online.de>

.github/workflows/build.yml

@@ -59,6 +59,10 @@ jobs:
         name: Prepare minimal package versions (MSRV only)
         run: nox -s set-minimal-package-versions
 
+      - if: inputs.rust == 'nightly' || inputs.msrv == 'MSRV'
+        name: Ignore changed error messages when using trybuild
+        run: echo "TRYBUILD=overwrite" >> "$GITHUB_ENV"
+
       - name: Build docs
         run: cargo doc --no-deps --no-default-features --features "full ${{ inputs.extra-features }}"
 

Cargo.toml

@@ -10,7 +10,7 @@ repository = "https://github.com/pyo3/pyo3"
 documentation = "https://docs.rs/crate/pyo3/"
 categories = ["api-bindings", "development-tools::ffi"]
 license = "Apache-2.0"
-exclude = ["/.gitignore", ".cargo/config", "/codecov.yml", "/Makefile", "/pyproject.toml", "/noxfile.py"]
+exclude = ["/.gitignore", ".cargo/config", "/codecov.yml", "/Makefile", "/pyproject.toml", "/noxfile.py", "/.github", "/tests/test_compile_error.rs", "/tests/ui"]
 edition = "2018"
 
 [dependencies]

src/marker.rs

@@ -140,6 +140,48 @@ use std::os::raw::c_int;
 /// the GIL is not held.
 ///
 /// See the [module-level documentation](self) for more information.
+///
+/// # Examples
+///
+/// This tracking is currently imprecise as it relies on the [`Send`] auto trait on stable Rust.
+/// For example, an `Rc` smart pointer should be usable without the GIL, but we currently prevent that:
+///
+/// ```compile_fail
+/// # use pyo3::prelude::*;
+/// use std::rc::Rc;
+///
+/// Python::with_gil(|py| {
+///     let rc = Rc::new(42);
+///
+///     py.allow_threads(|| {
+///         println!("{:?}", rc);
+///     });
+/// });
+/// ```
+///
+/// This also implies that the interplay between `with_gil` and `allow_threads` is unsound, for example
+/// one can circumvent this protection using the [`send_wrapper`](https://docs.rs/send_wrapper/) crate:
+///
+/// ```no_run
+/// # use pyo3::prelude::*;
+/// # use pyo3::types::PyString;
+/// use send_wrapper::SendWrapper;
+///
+/// Python::with_gil(|py| {
+///     let string = PyString::new(py, "foo");
+///
+///     let wrapped = SendWrapper::new(string);
+///
+///     py.allow_threads(|| {
+///         let sneaky: &PyString = *wrapped;
+///
+///         println!("{:?}", sneaky);
+///     });
+/// });
+/// ```
+///
+/// Fixing this loophole on stable Rust has significant ergonomic issues, but it is fixed when using
+/// nightly Rust and the `nightly` feature, c.f. [#2141](https://github.com/PyO3/pyo3/issues/2141).
 #[cfg_attr(docsrs, doc(cfg(all())))] // Hide the cfg flag
 #[cfg(not(feature = "nightly"))]
 pub unsafe trait Ungil {}
@@ -156,6 +198,70 @@ unsafe impl<T: Send> Ungil for T {}
 /// the GIL is not held.
 ///
 /// See the [module-level documentation](self) for more information.
+///
+/// # Examples
+///
+/// Types which are `Ungil` cannot be used in contexts where the GIL was released, e.g.
+///
+/// ```compile_fail
+/// # use pyo3::prelude::*;
+/// # use pyo3::types::PyString;
+/// Python::with_gil(|py| {
+///     let string = PyString::new(py, "foo");
+///
+///     py.allow_threads(|| {
+///         println!("{:?}", string);
+///     });
+/// });
+/// ```
+///
+/// This applies to the GIL token `Python` itself as well, e.g.
+///
+/// ```compile_fail
+/// # use pyo3::prelude::*;
+/// Python::with_gil(|py| {
+///     py.allow_threads(|| {
+///         drop(py);
+///     });
+/// });
+/// ```
+///
+/// On nightly Rust, this is not based on the [`Send`] auto trait and hence we are able
+/// to prevent incorrectly circumventing it using e.g. the [`send_wrapper`](https://docs.rs/send_wrapper/) crate:
+///
+/// ```compile_fail
+/// # use pyo3::prelude::*;
+/// # use pyo3::types::PyString;
+/// use send_wrapper::SendWrapper;
+///
+/// Python::with_gil(|py| {
+///     let string = PyString::new(py, "foo");
+///
+///     let wrapped = SendWrapper::new(string);
+///
+///     py.allow_threads(|| {
+///         let sneaky: &PyString = *wrapped;
+///
+///         println!("{:?}", sneaky);
+///     });
+/// });
+/// ```
+///
+/// This also enables using non-[`Send`] types in `allow_threads`,
+/// at least if they are not also bound to the GIL:
+///
+/// ```rust
+/// # use pyo3::prelude::*;
+/// use std::rc::Rc;
+///
+/// Python::with_gil(|py| {
+///     let rc = Rc::new(42);
+///
+///     py.allow_threads(|| {
+///         println!("{:?}", rc);
+///     });
+/// });
+/// ```
 #[cfg(feature = "nightly")]
 pub unsafe auto trait Ungil {}
 

tests/test_compile_error.rs

@@ -1,35 +1,8 @@
 #![cfg(feature = "macros")]
 
-#[rustversion::not(nightly)]
 #[cfg(not(target_arch = "wasm32"))] // Not possible to invoke compiler from wasm
 #[test]
 fn test_compile_errors() {
-    // stable - require all tests to pass
-    _test_compile_errors()
-}
-
-#[cfg(not(feature = "nightly"))]
-#[cfg(not(target_arch = "wasm32"))] // We are building wasm Python with pthreads disabled
-#[rustversion::nightly]
-#[test]
-fn test_compile_errors() {
-    // nightly - don't care if test output is potentially wrong, to avoid churn in PyO3's CI thanks
-    // to diagnostics changing on nightly.
-    let _ = std::panic::catch_unwind(_test_compile_errors);
-}
-
-#[cfg(feature = "nightly")]
-#[cfg(not(target_arch = "wasm32"))] // Not possible to invoke compiler from wasm
-#[rustversion::nightly]
-#[test]
-fn test_compile_errors() {
-    // nightly - don't care if test output is potentially wrong, to avoid churn in PyO3's CI thanks
-    // to diagnostics changing on nightly.
-    _test_compile_errors()
-}
-
-#[cfg(not(feature = "nightly"))]
-fn _test_compile_errors() {
     let t = trybuild::TestCases::new();
 
     t.compile_fail("tests/ui/invalid_macro_args.rs");
@@ -45,91 +18,24 @@ fn _test_compile_errors() {
     t.compile_fail("tests/ui/invalid_pymethod_names.rs");
     t.compile_fail("tests/ui/invalid_pymodule_args.rs");
     t.compile_fail("tests/ui/reject_generics.rs");
-
-    tests_rust_1_49(&t);
-    tests_rust_1_56(&t);
-    tests_rust_1_57(&t);
-    tests_rust_1_58(&t);
-    tests_rust_1_60(&t);
-    tests_rust_1_62(&t);
-    tests_rust_1_63(&t);
-
-    #[rustversion::since(1.49)]
-    fn tests_rust_1_49(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/deprecations.rs");
-    }
-    #[rustversion::before(1.49)]
-    fn tests_rust_1_49(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.56)]
-    fn tests_rust_1_56(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_closure.rs");
-
-        t.compile_fail("tests/ui/pyclass_send.rs");
-    }
-
-    #[rustversion::before(1.56)]
-    fn tests_rust_1_56(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.57)]
-    fn tests_rust_1_57(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_argument_attributes.rs");
-        t.compile_fail("tests/ui/invalid_frompy_derive.rs");
-        t.compile_fail("tests/ui/static_ref.rs");
-        t.compile_fail("tests/ui/wrong_aspyref_lifetimes.rs");
-    }
-
-    #[rustversion::before(1.57)]
-    fn tests_rust_1_57(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.58)]
-    fn tests_rust_1_58(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_pyfunctions.rs");
-        t.compile_fail("tests/ui/invalid_pymethods.rs");
-        #[cfg(Py_LIMITED_API)]
-        t.compile_fail("tests/ui/abi3_nativetype_inheritance.rs");
-    }
-
-    #[rustversion::before(1.58)]
-    fn tests_rust_1_58(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.60)]
-    fn tests_rust_1_60(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_intern_arg.rs");
-        t.compile_fail("tests/ui/invalid_frozen_pyclass_borrow.rs");
-    }
-
-    #[rustversion::before(1.60)]
-    fn tests_rust_1_60(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.62)]
-    fn tests_rust_1_62(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_pymethod_receiver.rs");
-        t.compile_fail("tests/ui/missing_intopy.rs");
-    }
-
-    #[rustversion::before(1.62)]
-    fn tests_rust_1_62(_t: &trybuild::TestCases) {}
-
-    #[rustversion::since(1.63)]
-    fn tests_rust_1_63(t: &trybuild::TestCases) {
-        t.compile_fail("tests/ui/invalid_result_conversion.rs");
-        t.compile_fail("tests/ui/not_send.rs");
-        t.compile_fail("tests/ui/not_send2.rs");
-        t.compile_fail("tests/ui/not_send3.rs");
-        t.compile_fail("tests/ui/get_set_all.rs");
-        t.compile_fail("tests/ui/traverse_bare_self.rs");
-    }
-
-    #[rustversion::before(1.63)]
-    fn tests_rust_1_63(_t: &trybuild::TestCases) {}
-}
-
-#[cfg(feature = "nightly")]
-fn _test_compile_errors() {
-    let t = trybuild::TestCases::new();
-
-    t.compile_fail("tests/ui/not_send_auto_trait.rs");
-    t.compile_fail("tests/ui/not_send_auto_trait2.rs");
-    t.compile_fail("tests/ui/send_wrapper.rs");
+    t.compile_fail("tests/ui/deprecations.rs");
+    t.compile_fail("tests/ui/invalid_closure.rs");
+    t.compile_fail("tests/ui/pyclass_send.rs");
+    t.compile_fail("tests/ui/invalid_argument_attributes.rs");
+    t.compile_fail("tests/ui/invalid_frompy_derive.rs");
+    t.compile_fail("tests/ui/static_ref.rs");
+    t.compile_fail("tests/ui/wrong_aspyref_lifetimes.rs");
+    t.compile_fail("tests/ui/invalid_pyfunctions.rs");
+    t.compile_fail("tests/ui/invalid_pymethods.rs");
+    #[cfg(Py_LIMITED_API)]
+    t.compile_fail("tests/ui/abi3_nativetype_inheritance.rs");
+    t.compile_fail("tests/ui/invalid_intern_arg.rs");
+    t.compile_fail("tests/ui/invalid_frozen_pyclass_borrow.rs");
+    t.compile_fail("tests/ui/invalid_pymethod_receiver.rs");
+    t.compile_fail("tests/ui/missing_intopy.rs");
+    t.compile_fail("tests/ui/invalid_result_conversion.rs");
+    t.compile_fail("tests/ui/not_send.rs");
+    t.compile_fail("tests/ui/not_send2.rs");
+    t.compile_fail("tests/ui/get_set_all.rs");
+    t.compile_fail("tests/ui/traverse_bare_self.rs");
 }