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.

Sign up for GitHub

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

Fix new clippy warnings from Rust 1.84 #13645

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Fix new clippy warnings from Rust 1.84 #13645

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8c3875
Fix new clippy warnings from Rust 1.84
mtreinish Jan 9, 2025
d55b564
Merge remote-tracking branch 'origin/main' into rust-clippy-84
mtreinish Jan 21, 2025
30e968e
Fix formatting
mtreinish Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions crates/accelerate/src/remove_diagonal_gates_before_measure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,15 @@ fn run_remove_diagonal_before_measure(dag: &mut DAGCircuit) -> PyResult<()> {
} else if DIAGONAL_2Q_GATES.contains(&gate)
|| DIAGONAL_3Q_GATES.contains(&gate)
{
let successors = dag.quantum_successors(predecessor);
let remove_s = successors
.map(|s| {
if let NodeType::Operation(inst_s) = &dag[s] {
inst_s.op.name() == "measure"
} else {
false
}
})
.all(|ok_to_remove| ok_to_remove);
if remove_s {
let mut successors = dag.quantum_successors(predecessor);
Copy link
Member

Choose a reason for hiding this comment

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

How come successors is mut now?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I thought this PR required more of successors, but actually it doesn't.

if successors.all(|s| {
let node_s = &dag.dag()[s];
if let NodeType::Operation(inst_s) = node_s {
inst_s.op.name() == "measure"
} else {
false
}
}) {
nodes_to_remove.push(predecessor);
}
}
Expand Down
Loading