Fix new clippy warnings from Rust 1.84 #13645
Conversation
The recently release Rust 1.84 introduced some new clippy rules that are triggering warnings in the Qiskit rust code. This commit fixes these warnings as the point to real issues most of the time. However in this release a bunch of the warnings are coming from a pyo3 macro that internally had logic for a feature flag that only exists in pyo3. I think this is likely a rule bug in clippy, but in the meantime adding a gil-refs feature to the qiskit crates to match the feature pyo3 is using silences the warnings.
mtreinish
added
type: qa
|
One or more of the following people are relevant to this code:
|
| }) | ||
| .all(|ok_to_remove| ok_to_remove); | ||
| if remove_s { | ||
| let mut successors = dag.quantum_successors(predecessor); |
There was a problem hiding this comment.
How come successors is mut now?
There was a problem hiding this comment.
It failed to compile if I didn't change it :) It looks like it's because .all() takes a mutable reference to self. It wasn't an issue before because the Map returned by the .map() call before was owned.
The error was:
error[E0596]: cannot borrow `successors` as mutable, as it is not declared as mutable
--> crates/accelerate/src/remove_diagonal_gates_before_measure.rs:70:32
|
70 | ... if successors.all(|s| {
| ^^^^^^^^^^ cannot borrow as mutable
|
help: consider changing this to be mutable
|
69 | let mut successors = dag.quantum_successors(predecessor);
| +++
For more information about this error, try `rustc --explain E0596`.
error: could not compile `qiskit-accelerate` (lib) due to 1 previous error
There was a problem hiding this comment.
Ah, I see - I was wondering how it could have compiled before. It's subtle: Iterator::map takes ownership, and you don't have to mark something mut to be able to transfer its ownership. Iterator::all "only" takes a mutable reference, so it was weird to me that you need mut when actually you're now using less permissions on the object, but really that assumption was a faulty abstraction in my head.
There was a problem hiding this comment.
I thought this PR required more of successors, but actually it doesn't.
Pull Request Test Coverage Report for Build 12698421954Details
💛 - Coveralls |
Summary
The recently release Rust 1.84 introduced some new clippy rules that are triggering warnings in the Qiskit rust code. This commit fixes these warnings as the point to real issues most of the time. However in this release a bunch of the warnings are coming from a pyo3 macro that internally had logic for a feature flag that only exists in pyo3. I think this is likely a rule bug in clippy, but in the meantime adding a gil-refs feature to the qiskit crates to match the feature pyo3 is using silences the warnings.
Details and comments