rust: updates for rust & clippy 1.54

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix regression in 0.14.0 rejecting usage of `#[doc(hidden)]` on structs and functions annotated with PyO3 macros. [#1722](https://github.com/PyO3/pyo3/pull/1722)
 - Fix regression in 0.14.0 leading to incorrect code coverage being computed for `#[pyfunction]`s. [#1726](https://github.com/PyO3/pyo3/pull/1726)
 - Fix incorrect FFI definition of `Py_Buffer` on PyPy. [#1737](https://github.com/PyO3/pyo3/pull/1737)
+- Fix incorrect calculation of `dictoffset` on 32-bit Windows. [#1475](https://github.com/PyO3/pyo3/pull/1475)
 
 ## [0.14.1] - 2021-07-04
 

Cargo.toml

@@ -32,7 +32,8 @@ serde = {version = "1.0", optional = true}
 
 [dev-dependencies]
 assert_approx_eq = "1.1.0"
-criterion = "0.3"
+# O.3.5 uses the matches! macro, which isn't compatible with Rust 1.41
+criterion = "=0.3.4"
 trybuild = "1.0.23"
 rustversion = "1.0"
 proptest = { version = "0.10.1", default-features = false, features = ["std"] }

benches/bench_call.rs

@@ -18,11 +18,11 @@ fn bench_call_0(b: &mut Bencher) {
         "#
         );
 
-        let foo = module.getattr("foo").unwrap();
+        let foo_module = module.getattr("foo").unwrap();
 
         b.iter(|| {
             for _ in 0..1000 {
-                foo.call0().unwrap();
+                foo_module.call0().unwrap();
             }
         });
     })
@@ -38,11 +38,11 @@ fn bench_call_method_0(b: &mut Bencher) {
         "#
         );
 
-        let foo = module.getattr("Foo").unwrap().call0().unwrap();
+        let foo_module = module.getattr("Foo").unwrap().call0().unwrap();
 
         b.iter(|| {
             for _ in 0..1000 {
-                foo.call_method0("foo").unwrap();
+                foo_module.call_method0("foo").unwrap();
             }
         });
     })

build.rs

@@ -108,7 +108,7 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<()
     match (is_extension_module, target_os.as_str()) {
         (_, "windows") => {
             // always link on windows, even with extension module
-            println!("{}", get_rustc_link_lib(&interpreter_config)?);
+            println!("{}", get_rustc_link_lib(interpreter_config)?);
             // Set during cross-compiling.
             if let Some(libdir) = &interpreter_config.libdir {
                 println!("cargo:rustc-link-search=native={}", libdir);
@@ -126,7 +126,7 @@ fn emit_cargo_configuration(interpreter_config: &InterpreterConfig) -> Result<()
         (false, _) | (_, "android") => {
             // other systems, only link libs if not extension module
             // android always link.
-            println!("{}", get_rustc_link_lib(&interpreter_config)?);
+            println!("{}", get_rustc_link_lib(interpreter_config)?);
             if let Some(libdir) = &interpreter_config.libdir {
                 println!("cargo:rustc-link-search=native={}", libdir);
             }
@@ -192,11 +192,18 @@ fn configure_pyo3() -> Result<()> {
     )?;
     interpreter_config.emit_pyo3_cfgs();
 
+    let rustc_minor_version = rustc_minor_version().unwrap_or(0);
+
     // Enable use of const generics on Rust 1.51 and greater
-    if rustc_minor_version().unwrap_or(0) >= 51 {
+    if rustc_minor_version >= 51 {
         println!("cargo:rustc-cfg=min_const_generics");
     }
 
+    // Enable use of std::ptr::addr_of! on Rust 1.51 and greater
+    if rustc_minor_version >= 51 {
+        println!("cargo:rustc-cfg=addr_of");
+    }
+
     Ok(())
 }
 

pyo3-build-config/src/impl_.rs

@@ -431,7 +431,7 @@ impl BuildFlags {
             script.push_str(&format!("print(config.get('{}', '0'))\n", k));
         }
 
-        let stdout = run_python_script(&interpreter, &script)?;
+        let stdout = run_python_script(interpreter, &script)?;
         let split_stdout: Vec<&str> = stdout.trim_end().lines().collect();
         ensure!(
             split_stdout.len() == BuildFlags::ALL.len(),
@@ -573,7 +573,7 @@ fn ends_with(entry: &DirEntry, pat: &str) -> bool {
 ///
 /// [1]: https://github.com/python/cpython/blob/3.5/Lib/sysconfig.py#L389
 fn find_sysconfigdata(cross: &CrossCompileConfig) -> Result<PathBuf> {
-    let sysconfig_paths = search_lib_dir(&cross.lib_dir, &cross);
+    let sysconfig_paths = search_lib_dir(&cross.lib_dir, cross);
     let sysconfig_name = env_var("_PYTHON_SYSCONFIGDATA_NAME");
     let mut sysconfig_paths = sysconfig_paths
         .iter()

pyo3-macros-backend/src/from_pyobject.rs

@@ -186,7 +186,7 @@ impl<'a> Container<'a> {
     /// Build derivation body for a struct.
     fn build(&self) -> TokenStream {
         match &self.ty {
-            ContainerType::StructNewtype(ident) => self.build_newtype_struct(Some(&ident)),
+            ContainerType::StructNewtype(ident) => self.build_newtype_struct(Some(ident)),
             ContainerType::TupleNewtype => self.build_newtype_struct(None),
             ContainerType::Tuple(len) => self.build_tuple_struct(*len),
             ContainerType::Struct(tups) => self.build_struct(tups),

pyo3-macros-backend/src/method.rs

@@ -266,7 +266,7 @@ impl<'a> FnSpec<'a> {
         let python_name = python_name.as_ref().unwrap_or(name).unraw();
 
         let doc = utils::get_doc(
-            &meth_attrs,
+            meth_attrs,
             options
                 .text_signature
                 .as_ref()
@@ -413,7 +413,7 @@ impl<'a> FnSpec<'a> {
                 Argument::Arg(path, opt) | Argument::Kwarg(path, opt) => {
                     if path.is_ident(name) {
                         if let Some(val) = opt {
-                            let i: syn::Expr = syn::parse_str(&val).unwrap();
+                            let i: syn::Expr = syn::parse_str(val).unwrap();
                             return Some(i.into_token_stream());
                         }
                     }

pyo3-macros-backend/src/params.rs

@@ -29,7 +29,7 @@ pub fn accept_args_kwargs(attrs: &[Argument]) -> (bool, bool) {
 
 /// Return true if the argument list is simply (*args, **kwds).
 pub fn is_forwarded_args(args: &[FnArg<'_>], attrs: &[Argument]) -> bool {
-    args.len() == 2 && is_args(attrs, &args[0].name) && is_kwargs(attrs, &args[1].name)
+    args.len() == 2 && is_args(attrs, args[0].name) && is_kwargs(attrs, args[1].name)
 }
 
 fn is_args(attrs: &[Argument], name: &syn::Ident) -> bool {
@@ -68,15 +68,7 @@ pub fn impl_arg_params(
         // is (*args, **kwds).
         let mut arg_convert = vec![];
         for (i, arg) in spec.args.iter().enumerate() {
-            arg_convert.push(impl_arg_param(
-                arg,
-                &spec,
-                i,
-                None,
-                &mut 0,
-                py,
-                &args_array,
-            )?);
+            arg_convert.push(impl_arg_param(arg, spec, i, None, &mut 0, py, &args_array)?);
         }
         return Ok(quote! {{
             let _args = Some(_args);
@@ -90,12 +82,12 @@ pub fn impl_arg_params(
     let mut keyword_only_parameters = Vec::new();
 
     for arg in spec.args.iter() {
-        if arg.py || is_args(&spec.attrs, &arg.name) || is_kwargs(&spec.attrs, &arg.name) {
+        if arg.py || is_args(&spec.attrs, arg.name) || is_kwargs(&spec.attrs, arg.name) {
             continue;
         }
         let name = arg.name.unraw().to_string();
-        let kwonly = spec.is_kw_only(&arg.name);
-        let required = !(arg.optional.is_some() || spec.default_value(&arg.name).is_some());
+        let kwonly = spec.is_kw_only(arg.name);
+        let required = !(arg.optional.is_some() || spec.default_value(arg.name).is_some());
 
         if kwonly {
             keyword_only_parameters.push(quote! {
@@ -118,8 +110,8 @@ pub fn impl_arg_params(
     let mut option_pos = 0;
     for (idx, arg) in spec.args.iter().enumerate() {
         param_conversion.push(impl_arg_param(
-            &arg,
-            &spec,
+            arg,
+            spec,
             idx,
             self_,
             &mut option_pos,
@@ -212,15 +204,15 @@ fn impl_arg_param(
         |e| pyo3::derive_utils::argument_extraction_error(#py, stringify!(#name), e)
     };
 
-    if is_args(&spec.attrs, &name) {
+    if is_args(&spec.attrs, name) {
         ensure_spanned!(
             arg.optional.is_none(),
             arg.name.span() => "args cannot be optional"
         );
         return Ok(quote_arg_span! {
             let #arg_name = _args.unwrap().extract().map_err(#transform_error)?;
         });
-    } else if is_kwargs(&spec.attrs, &name) {
+    } else if is_kwargs(&spec.attrs, name) {
         ensure_spanned!(
             arg.optional.is_some(),
             arg.name.span() => "kwargs must be Option<_>"
@@ -259,7 +251,7 @@ fn impl_arg_param(
         }
     };
 
-    return if let syn::Type::Reference(tref) = unwrap_ty_group(arg.optional.unwrap_or(&ty)) {
+    return if let syn::Type::Reference(tref) = unwrap_ty_group(arg.optional.unwrap_or(ty)) {
         let (tref, mut_) = preprocess_tref(tref, self_);
         let (target_ty, borrow_tmp) = if arg.optional.is_some() {
             // Get Option<&T> from Option<PyRef<T>>

pyo3-macros-backend/src/pyclass.rs

@@ -262,7 +262,7 @@ pub fn build_py_class(
 
     impl_class(
         &class.ident,
-        &args,
+        args,
         doc,
         field_options,
         methods_type,
@@ -452,7 +452,7 @@ fn impl_class(
     let (impl_inventory, for_each_py_method) = match methods_type {
         PyClassMethodsType::Specialization => (None, quote! { visitor(collector.py_methods()); }),
         PyClassMethodsType::Inventory => (
-            Some(impl_methods_inventory(&cls)),
+            Some(impl_methods_inventory(cls)),
             quote! {
                 for inventory in pyo3::inventory::iter::<<Self as pyo3::class::impl_::HasMethodsInventory>::Methods>() {
                     visitor(pyo3::class::impl_::PyMethodsInventory::get(inventory));

pyo3-macros-backend/src/pymethod.rs

@@ -303,7 +303,7 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType) -> Resul
 fn split_off_python_arg<'a>(args: &'a [FnArg<'a>]) -> (Option<&FnArg>, &[FnArg]) {
     if args
         .get(0)
-        .map(|py| utils::is_python(&py.ty))
+        .map(|py| utils::is_python(py.ty))
         .unwrap_or(false)
     {
         (Some(&args[0]), &args[1..])

src/buffer.rs

@@ -666,7 +666,7 @@ mod tests {
         let gil = Python::acquire_gil();
         let py = gil.python();
         let bytes = py.eval("b'abcde'", None, None).unwrap();
-        let buffer = PyBuffer::get(&bytes).unwrap();
+        let buffer = PyBuffer::get(bytes).unwrap();
         assert_eq!(buffer.dimensions(), 1);
         assert_eq!(buffer.item_count(), 5);
         assert_eq!(buffer.format().to_str().unwrap(), "B");

src/conversions/osstr.rs

@@ -121,7 +121,7 @@ impl IntoPy<PyObject> for &'_ OsStr {
 impl ToPyObject for Cow<'_, OsStr> {
     #[inline]
     fn to_object(&self, py: Python) -> PyObject {
-        (&self as &OsStr).to_object(py)
+        (self as &OsStr).to_object(py)
     }
 }
 
@@ -135,7 +135,7 @@ impl IntoPy<PyObject> for Cow<'_, OsStr> {
 impl ToPyObject for OsString {
     #[inline]
     fn to_object(&self, py: Python) -> PyObject {
-        (&self as &OsStr).to_object(py)
+        (self as &OsStr).to_object(py)
     }
 }
 

src/exceptions.rs

@@ -402,15 +402,15 @@ mod tests {
         let error_type = py.get_type::<CustomError>();
         let ctx = [("CustomError", error_type)].into_py_dict(py);
         let type_description: String = py
-            .eval("str(CustomError)", None, Some(&ctx))
+            .eval("str(CustomError)", None, Some(ctx))
             .unwrap()
             .extract()
             .unwrap();
         assert_eq!(type_description, "<class 'mymodule.CustomError'>");
         py.run(
             "assert CustomError('oops').args == ('oops',)",
             None,
-            Some(&ctx),
+            Some(ctx),
         )
         .unwrap();
     }

src/pycell.rs

@@ -104,34 +104,84 @@ pub struct PyCell<T: PyClass> {
 pub(crate) struct PyCellContents<T: PyClass> {
     pub(crate) value: ManuallyDrop<UnsafeCell<T>>,
     pub(crate) thread_checker: T::ThreadChecker,
-    // DO NOT CHANGE THE ORDER OF THESE FIELDS WITHOUT CHANGING PyCell::dict_offset()
-    // AND PyCell::weakref_offset()
     pub(crate) dict: T::Dict,
     pub(crate) weakref: T::WeakRef,
 }
 
 impl<T: PyClass> PyCell<T> {
     /// Get the offset of the dictionary from the start of the struct in bytes.
     #[cfg(not(all(Py_LIMITED_API, not(Py_3_9))))]
-    pub(crate) fn dict_offset() -> Option<usize> {
+    pub(crate) fn dict_offset() -> Option<ffi::Py_ssize_t> {
+        use std::convert::TryInto;
         if T::Dict::IS_DUMMY {
             None
         } else {
-            Some(
-                std::mem::size_of::<Self>()
-                    - std::mem::size_of::<T::Dict>()
-                    - std::mem::size_of::<T::WeakRef>(),
-            )
+            #[cfg(addr_of)]
+            let offset = {
+                // With std::ptr::addr_of - can measure offset using uninit memory without UB.
+                let cell = std::mem::MaybeUninit::<Self>::uninit();
+                let base_ptr = cell.as_ptr();
+                let dict_ptr = unsafe { std::ptr::addr_of!((*base_ptr).contents.dict) };
+                unsafe { (dict_ptr as *const u8).offset_from(base_ptr as *const u8) }
+            };
+            #[cfg(not(addr_of))]
+            let offset = {
+                // No std::ptr::addr_of - need to take references to PyCell to measure offsets;
+                // make a zero-initialised "fake" one so that referencing it is not UB.
+                let mut cell = std::mem::MaybeUninit::<Self>::uninit();
+                unsafe {
+                    std::ptr::write_bytes(cell.as_mut_ptr(), 0, 1);
+                }
+                let cell = unsafe { cell.assume_init() };
+                let dict_ptr = &cell.contents.dict;
+                // offset_from wasn't stabilised until 1.47, so we also have to work around
+                // that...
+                let offset = (dict_ptr as *const _ as usize) - (&cell as *const _ as usize);
+                // This isn't a valid cell, so ensure no Drop code runs etc.
+                std::mem::forget(cell);
+                offset
+            };
+            // Py_ssize_t may not be equal to isize on all platforms
+            #[allow(clippy::useless_conversion)]
+            Some(offset.try_into().expect("offset should fit in Py_ssize_t"))
         }
     }
 
     /// Get the offset of the weakref list from the start of the struct in bytes.
     #[cfg(not(all(Py_LIMITED_API, not(Py_3_9))))]
-    pub(crate) fn weakref_offset() -> Option<usize> {
+    pub(crate) fn weakref_offset() -> Option<ffi::Py_ssize_t> {
+        use std::convert::TryInto;
         if T::WeakRef::IS_DUMMY {
             None
         } else {
-            Some(std::mem::size_of::<Self>() - std::mem::size_of::<T::WeakRef>())
+            #[cfg(addr_of)]
+            let offset = {
+                // With std::ptr::addr_of - can measure offset using uninit memory without UB.
+                let cell = std::mem::MaybeUninit::<Self>::uninit();
+                let base_ptr = cell.as_ptr();
+                let weaklist_ptr = unsafe { std::ptr::addr_of!((*base_ptr).contents.weakref) };
+                unsafe { (weaklist_ptr as *const u8).offset_from(base_ptr as *const u8) }
+            };
+            #[cfg(not(addr_of))]
+            let offset = {
+                // No std::ptr::addr_of - need to take references to PyCell to measure offsets;
+                // make a zero-initialised "fake" one so that referencing it is not UB.
+                let mut cell = std::mem::MaybeUninit::<Self>::uninit();
+                unsafe {
+                    std::ptr::write_bytes(cell.as_mut_ptr(), 0, 1);
+                }
+                let cell = unsafe { cell.assume_init() };
+                let weaklist_ptr = &cell.contents.weakref;
+                // offset_from wasn't stabilised until 1.47, so we also have to work around
+                // that...
+                let offset = (weaklist_ptr as *const _ as usize) - (&cell as *const _ as usize);
+                // This isn't a valid cell, so ensure no Drop code runs etc.
+                std::mem::forget(cell);
+                offset
+            };
+            // Py_ssize_t may not be equal to isize on all platforms
+            #[allow(clippy::useless_conversion)]
+            Some(offset.try_into().expect("offset should fit in Py_ssize_t"))
         }
     }
 }