Applied several clippy suggestions

[ZapAnton]

This PR tries to reduce the warning-noise when cargo clippy is applied.

The most common warning was about passing arguments by value instead of passing by reference:

warning: this argument is passed by value, but not consumed in the function body
   --> src/lib.rs:228:8
    |
228 |     s: PyObject,
    |        ^^^^^^^^ help: consider taking a reference instead: `&PyObject`
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#needless_pass_by_value

Some of the warnings I managed to resolve, but for functions annotated with #[pyfunction] the simple usage of &Option<> introduces an error:

error[E0277]: the trait bound `pyo3::PyObject: pyo3::PyTypeInfo` is not satisfied                                                                                                             
   --> src/lib.rs:153:1                                                                                                                                                                       
    |                                                                                                                                                                                         
153 | #[pyfunction]                                                                                                                                                                           
    | ^^^^^^^^^^^^^ the trait `pyo3::PyTypeInfo` is not implemented for `pyo3::PyObject`                                                                                                      
    |                                                                                                                                                                                         
    = note: required because of the requirements on the impl of `pyo3::PyTryFrom` for `pyo3::PyObject`                                                                                        
    = note: required because of the requirements on the impl of `pyo3::FromPyObject<'_>` for `&pyo3::PyObject`                                                                                
    = note: required by `pyo3::ObjectProtocol::extract`   

The way to resolve the error could be the usage the PyObjectRef and also the usage of #[pyfn()] instead of #[pyfunction] (actually could not find #[pyfunction] in the 0.5.0 documentation - could it be deprecated?), but that would introduce the API breakage, so I left it for now.

The result changes are somewhat chaotic, so feel free to close

[dtolnay]

Not affiliated with hyperjson but just my opinion: I have not found the needless_pass_by_value lint to be valuable and always just disable it in my projects when it comes up.

#![allow(clippy::needless_pass_by_value)]

In this case PyObject is already conceptually and implementation-wise a pointer, so writing things in terms of &PyObject rather than PyObject does not seem useful.

[ZapAnton]

The elimination of needless_pass_by_value actually made me to change some arguments from &Option<Value> into Option<&Value> which clippy claimed could be somewhat faster, so maybe there is some merit in it.

[dtolnay]

If something is already a pointer (like PyObject) then sticking it behind another pointer is likely to make things slower, not faster.

See for example https://rust.godbolt.org/z/R7NKYZ in which the Option<&PyObject> requires more instructions.

[mre]

Looking at the godbolt link I lean towards what @dtolnay says.
If it's not slower, I'd keep the pass-by-value; I find it more readable.

@ZapAnton, could you try adding the clippy setting and revert the references?

#![allow(clippy::needless_pass_by_value)]

actually could not find #[pyfunction] in the 0.5.0 documentation - could it be deprecated?

Hum? I see it being used here: https://pyo3.rs/v0.5.0/

The way to resolve the error could be the usage the PyObjectRef and also the usage of #[pyfn()] instead of #[pyfunction]

I think #[pyfunction] is still the way to go as #[pyfn()] is solely used for methods in the code.

Would be interested in benchmarking PyObject vs PyObjectRef, though.

[dtolnay]

Meanwhile I opened rust-lang/rust-clippy#3439 to disable that needless_pass_by_value lint by default in Clippy.

[konstin]

The way to resolve the error could be the usage the PyObjectRef and also the usage of #[pyfn()] instead of #[pyfunction]

pyfn and pyfunction use the same code generation backend, so it's unlikely the cause.

[mre]

@konstin, do you think that testing PyObject vs PyObjectRef makes sense? It shouldn't have any perf impact or make a difference in the number of allocations or?

[konstin]

I've actually no idea how they compare regarding performance