Skip to content
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

RFC: Simplify version handling of UI tests. #3171

Merged
merged 6 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved

- name: Build docs
run: cargo doc --no-deps --no-default-features --features "full ${{ inputs.extra-features }}"

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
106 changes: 106 additions & 0 deletions src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand All @@ -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 {}

Expand Down
134 changes: 20 additions & 114 deletions tests/test_compile_error.rs
Original file line number Diff line number Diff line change
@@ -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");
Expand All @@ -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");
}
12 changes: 0 additions & 12 deletions tests/ui/not_send3.rs

This file was deleted.

24 changes: 0 additions & 24 deletions tests/ui/not_send3.stderr

This file was deleted.

11 changes: 0 additions & 11 deletions tests/ui/not_send_auto_trait.rs

This file was deleted.

21 changes: 0 additions & 21 deletions tests/ui/not_send_auto_trait.stderr

This file was deleted.

12 changes: 0 additions & 12 deletions tests/ui/not_send_auto_trait2.rs

This file was deleted.