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

Restore compatibility with Rust 1.41(.1). #1421

Merged
merged 1 commit into from Feb 11, 2021

Conversation

birkenfeld
Copy link
Member

@birkenfeld birkenfeld commented Feb 10, 2021

This version is currently supported by Debian stable and Alpine Linux.

Fixes #1420

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It's a shame to give up the ergonomics of the newer compiler. However I think making the adoption of cryptography into as many packaging solutions as possible is worth the (admittedly still small) costs to us.

Could you please add this into the CHANGELOG in a ## Packaging section at the top? (Similar to the 0.13 release ordering.)

@@ -115,6 +115,7 @@ pub fn parse_method_receiver(arg: &syn::FnArg) -> syn::Result<SelfType> {

impl<'a> FnSpec<'a> {
/// Parser function signature and function attributes
#[allow(clippy::manual_strip)] // for strip_prefix replacement below
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[allow(clippy::manual_strip)] // for strip_prefix replacement below
#[allow(clippy::manual_strip)] // for strip_prefix replacement supporting rust < 1.45

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

README.md Outdated
@@ -18,7 +18,7 @@ A comparison with rust-cpython can be found [in the guide](https://pyo3.rs/maste

## Usage

PyO3 supports Python 3.6 and up. The minimum required Rust version is 1.45.0.
PyO3 supports Python 3.6 and up. The minimum required Rust version is 1.41.1.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo patch releases probably aren't that interesting unless we do actually depend on the patch fix. Shall we just use major.minor here?

Suggested change
PyO3 supports Python 3.6 and up. The minimum required Rust version is 1.41.1.
PyO3 supports Python 3.6 and up. The minimum required Rust version is 1.41.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@davidhewitt
Copy link
Member

Regarding UI tests, we use #[rustversion] to move problematic test cases into gated functions. See test_compile_error.rs.

I suggest moving anything that doesn't pass on Rust 1.41 into a tests_rust_1_45 function.

This version is currently supported by Debian stable and Alpine Linux.

Fixes PyO3#1420
Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 😉

#[cfg(Py_3_8)]
if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
ffi::Py_DECREF(ty as *mut ffi::PyObject);
if cfg!(Py_3_8) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you prefer this instead of if cfg!(Py_3_8) && .. ?
Or, if you want to avoid #[allow(clippy)] you can use

            cfg_if::cfg_if! {
                if #[cfg(Py_3_8)] {
                    if ffi::PyType_HasFeature(ty, ffi::Py_TPFLAGS_HEAPTYPE) != 0 {
                        ffi::Py_DECREF(ty as *mut ffi::PyObject);
                    }
                }
            }

Though I'm not sure which is bettter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of later, when the #[cfg] form can be used again, it is easier to replace this back.

In general I hate the "collapsible_if" lint, since it is in at least 50% of cases not an oversight and makes the code clearer.

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This looks great to me, thanks for fixing up!

Will merge this and rebase the patch release.

@davidhewitt davidhewitt merged commit 83f71d8 into PyO3:master Feb 11, 2021
Comment on lines +51 to +56
[modifier, char]
if (*modifier == b'='
|| *modifier == b'<'
|| *modifier == b'>'
|| *modifier == b'!') =>
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[modifier, char]
if (*modifier == b'='
|| *modifier == b'<'
|| *modifier == b'>'
|| *modifier == b'!') =>
{
[b'=', char] | [b'<', char] | [b'>', char] | [b'!', char] => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for older Rust versions (1.41)?
4 participants