From e6a00846880f4453806b76aab4a60a18d1adbab7 Mon Sep 17 00:00:00 2001 From: Arthur Bit-Monnot Date: Fri, 14 Jun 2024 23:12:42 +0200 Subject: [PATCH] lint: fix clippy lint for rust 1.79 --- .github/workflows/aries.yml | 172 +++++++++--------- planning/grpc/server/src/bin/server.rs | 31 ++-- planning/grpc/server/src/chronicles.rs | 2 +- planning/planners/src/encode/symmetry.rs | 4 +- .../analysis/detrimental_supports.rs | 2 +- .../src/chronicles/analysis/static_fluents.rs | 7 +- .../preprocessing/action_rolling.rs | 4 +- solver/src/collections/ref_store.rs | 4 +- solver/src/model/model_impl.rs | 1 - solver/src/model/symbols.rs | 4 +- solver/src/model/types.rs | 4 +- solver/src/reasoners/eq/dense.rs | 31 ++-- solver/src/reasoners/eq/domain.rs | 2 +- solver/src/reasoners/eq/split.rs | 2 +- .../interfaces/unified_planning/expression.rs | 2 +- .../src/interfaces/unified_planning/mod.rs | 7 +- 16 files changed, 134 insertions(+), 145 deletions(-) diff --git a/.github/workflows/aries.yml b/.github/workflows/aries.yml index 1a814c1e..42dfd16b 100644 --- a/.github/workflows/aries.yml +++ b/.github/workflows/aries.yml @@ -23,7 +23,7 @@ jobs: - uses: actions/checkout@v3 with: fetch-depth: 0 - - uses: dtolnay/rust-toolchain@1.75.0 # fixed version to avoid failures on rust version releases + - uses: dtolnay/rust-toolchain@1.79.0 # fixed version to avoid failures on rust version releases with: components: clippy, rustfmt - run: cargo fmt --all -- --check @@ -96,24 +96,24 @@ jobs: run: | source planning/unified/dev.env python3 planning/unified/deps/unified-planning/up_test_cases/report.py aries - + tests: # Meta-job that only requires all test-jobs to pass - needs: [lints, unit-tests, integration-tests, unified-planning-api, unified-planning-integration] + needs: [ lints, unit-tests, integration-tests, unified-planning-api, unified-planning-integration ] runs-on: ubuntu-latest steps: - run: true -# ================ Building & Releasing binaries. -# Only active on the master branch and when the previous validation steps passed + # ================ Building & Releasing binaries. + # Only active on the master branch and when the previous validation steps passed build: # Build release binaries for all architecture and save them as artifacts strategy: matrix: - os: [ubuntu-latest, macos-latest, windows-latest] - target: [amd64, arm64] + os: [ ubuntu-latest, macos-latest, windows-latest ] + target: [ amd64, arm64 ] exclude: - os: ubuntu-latest target: arm64 # linux-arm64 build has linker issues, done in a distinct job @@ -166,95 +166,95 @@ jobs: path: bins/${{ env.BINARY }} retention-days: 1 -# Deactivated as induces very long CI times -# # Build linux-aarch64 binaries in a dedicated container. -# build-linux-arm64: -# runs-on: ubuntu-latest -# name: Build - ubuntu-latest - arm64 -# if: github.ref == 'refs/heads/master' -# needs: [lints, unit-tests, integration-tests, unified-planning-api] -# steps: -# - uses: actions/checkout@v3 -# with: -# fetch-depth: 0 -# - uses: uraimo/run-on-arch-action@v2 -# name: Build on ubuntu-latest targetting ARM64 -# id: build -# with: -# arch: aarch64 -# distro: ubuntu20.04 -# -# githubToken: ${{ secrets.GITHUB_TOKEN }} -# -# dockerRunArgs: | -# --privileged --volume "${PWD}:/workdir" --workdir /workdir -# -# shell: /bin/bash -# setup: mkdir -p bins/ -# install: | -# apt-get update -# apt-get -y upgrade -# apt-get install -y libssl-dev libudev-dev pkg-config curl git -# apt-get install -y build-essential gcc-aarch64-linux-gnu python3.8 -# curl https://sh.rustup.rs -sSf | sh -s -- -y -# echo $HOME/.cargo/bin >> ~/.bashrc -# source $HOME/.cargo/env -# rustup target add aarch64-unknown-linux-gnu -# run: | -# source $HOME/.cargo/env -# cargo build --release --target aarch64-unknown-linux-gnu --bin up-server -# cp target/aarch64-unknown-linux-gnu/release/up-server bins/up-aries_linux_arm64 -# python3.8 ./ci/grpc.py --executable bins/up-aries_linux_arm64 -# -# - name: Upload artifact -# uses: actions/upload-artifact@v2 -# with: -# name: up-aries_linux_arm64 -# path: bins/up-aries_linux_arm64 -# retention-days: 1 + # Deactivated as induces very long CI times + # # Build linux-aarch64 binaries in a dedicated container. + # build-linux-arm64: + # runs-on: ubuntu-latest + # name: Build - ubuntu-latest - arm64 + # if: github.ref == 'refs/heads/master' + # needs: [lints, unit-tests, integration-tests, unified-planning-api] + # steps: + # - uses: actions/checkout@v3 + # with: + # fetch-depth: 0 + # - uses: uraimo/run-on-arch-action@v2 + # name: Build on ubuntu-latest targetting ARM64 + # id: build + # with: + # arch: aarch64 + # distro: ubuntu20.04 + # + # githubToken: ${{ secrets.GITHUB_TOKEN }} + # + # dockerRunArgs: | + # --privileged --volume "${PWD}:/workdir" --workdir /workdir + # + # shell: /bin/bash + # setup: mkdir -p bins/ + # install: | + # apt-get update + # apt-get -y upgrade + # apt-get install -y libssl-dev libudev-dev pkg-config curl git + # apt-get install -y build-essential gcc-aarch64-linux-gnu python3.8 + # curl https://sh.rustup.rs -sSf | sh -s -- -y + # echo $HOME/.cargo/bin >> ~/.bashrc + # source $HOME/.cargo/env + # rustup target add aarch64-unknown-linux-gnu + # run: | + # source $HOME/.cargo/env + # cargo build --release --target aarch64-unknown-linux-gnu --bin up-server + # cp target/aarch64-unknown-linux-gnu/release/up-server bins/up-aries_linux_arm64 + # python3.8 ./ci/grpc.py --executable bins/up-aries_linux_arm64 + # + # - name: Upload artifact + # uses: actions/upload-artifact@v2 + # with: + # name: up-aries_linux_arm64 + # path: bins/up-aries_linux_arm64 + # retention-days: 1 package-python: name: Python Package (up_aries) - needs: [build] + needs: [ build ] runs-on: ubuntu-latest if: (github.ref == 'refs/heads/master') || startsWith(github.ref, 'refs/tags/v') steps: - - uses: actions/checkout@v3 - - name: Retrieve git tags # need full git history to determine version number when packaging - run: git fetch --prune --unshallow - - name: Available tags - run: | - git tag - git describe --tags --match v[0-9]* - - uses: actions/setup-python@v4 - with: - python-version: '3.8' - - name: Install build tools - run: pip install build - - uses: actions/download-artifact@v3 - with: - path: planning/unified/plugin/artifacts - - name: Unpack artifacts - run: | - cd planning/unified/plugin/ - ls -lR - mkdir -p up_aries/bin - cp artifacts/*/* up_aries/bin/ - chmod +x up_aries/bin/* - ls -lR - python -m build --sdist - cp dist/up_aries-*.tar.gz up_aries.tar.gz - - uses: actions/upload-artifact@v3 - with: - name: up_aries.tar.gz - path: planning/unified/plugin/up_aries.tar.gz - retention-days: 1 + - uses: actions/checkout@v3 + - name: Retrieve git tags # need full git history to determine version number when packaging + run: git fetch --prune --unshallow + - name: Available tags + run: | + git tag + git describe --tags --match v[0-9]* + - uses: actions/setup-python@v4 + with: + python-version: '3.8' + - name: Install build tools + run: pip install build + - uses: actions/download-artifact@v3 + with: + path: planning/unified/plugin/artifacts + - name: Unpack artifacts + run: | + cd planning/unified/plugin/ + ls -lR + mkdir -p up_aries/bin + cp artifacts/*/* up_aries/bin/ + chmod +x up_aries/bin/* + ls -lR + python -m build --sdist + cp dist/up_aries-*.tar.gz up_aries.tar.gz + - uses: actions/upload-artifact@v3 + with: + name: up_aries.tar.gz + path: planning/unified/plugin/up_aries.tar.gz + retention-days: 1 - pre-release: # If on master branch, Upload all artifacts as a pre-release "latest" + pre-release: # If on master branch, Upload all artifacts as a pre-release "latest" name: Pre Release runs-on: ubuntu-latest if: github.ref == 'refs/heads/master' - needs: [tests, build, package-python] + needs: [ tests, build, package-python ] steps: - name: Download artifact uses: actions/download-artifact@v2 diff --git a/planning/grpc/server/src/bin/server.rs b/planning/grpc/server/src/bin/server.rs index 2ea7041e..bae2eecf 100644 --- a/planning/grpc/server/src/bin/server.rs +++ b/planning/grpc/server/src/bin/server.rs @@ -352,24 +352,21 @@ impl UnifiedPlanning for UnifiedPlanningService { let problem = Arc::new(problem); let result = solve(problem, |_| {}, conf).await; - let mut answer = match result { - Ok(answer) => answer, - Err(e) => { - let message = format!("{}", e.chain().rev().format("\n Context: ")); - eprintln!("ERROR: {}", &message); - let log_message = LogMessage { - level: log_message::LogLevel::Error as i32, - message, - }; - PlanGenerationResult { - status: plan_generation_result::Status::InternalError as i32, - plan: None, - metrics: Default::default(), - log_messages: vec![log_message], - engine: Some(engine()), - } + let mut answer = result.unwrap_or_else(|e| { + let message = format!("{}", e.chain().rev().format("\n Context: ")); + eprintln!("ERROR: {}", &message); + let log_message = LogMessage { + level: log_message::LogLevel::Error as i32, + message, + }; + PlanGenerationResult { + status: plan_generation_result::Status::InternalError as i32, + plan: None, + metrics: Default::default(), + log_messages: vec![log_message], + engine: Some(engine()), } - }; + }); add_engine_time(&mut answer.metrics, &reception_time); Ok(Response::new(answer)) } diff --git a/planning/grpc/server/src/chronicles.rs b/planning/grpc/server/src/chronicles.rs index 18d93cfb..4e4b19a9 100644 --- a/planning/grpc/server/src/chronicles.rs +++ b/planning/grpc/server/src/chronicles.rs @@ -765,7 +765,7 @@ impl<'a> ChronicleFactory<'a> { } /// Final value to minimize is converted to a condition at the chronicle end time - fn add_final_value_metric(&mut self, metrics: &Vec) -> Result<(), Error> { + fn add_final_value_metric(&mut self, metrics: &[Metric]) -> Result<(), Error> { ensure!(metrics.len() <= 1, "Unsupported: multiple metrics provided."); let Some(metric) = metrics.first() else { return Ok(()); // no metric to handle diff --git a/planning/planners/src/encode/symmetry.rs b/planning/planners/src/encode/symmetry.rs index 6cd546ec..e26c74f5 100644 --- a/planning/planners/src/encode/symmetry.rs +++ b/planning/planners/src/encode/symmetry.rs @@ -217,8 +217,8 @@ fn add_plan_space_symmetry_breaking(pb: &FiniteProblem, model: &mut Model, encod clause.clear(); clause.push(!supports(*instance, *cond)); - for prev_cond_index in 0..cond_index { - clause.push(supports(*prev, conditions[prev_cond_index])); + for prev_cond in &conditions[0..cond_index] { + clause.push(supports(*prev, *prev_cond)); } model.enforce(or(clause.as_slice()), []); } diff --git a/planning/planning/src/chronicles/analysis/detrimental_supports.rs b/planning/planning/src/chronicles/analysis/detrimental_supports.rs index 63fa6731..4fc3e098 100644 --- a/planning/planning/src/chronicles/analysis/detrimental_supports.rs +++ b/planning/planning/src/chronicles/analysis/detrimental_supports.rs @@ -194,7 +194,7 @@ impl CausalSupport { }, condition: TemplateCondID { template_id: cond_template, - cond_id: cond_id, + cond_id, }, } } diff --git a/planning/planning/src/chronicles/analysis/static_fluents.rs b/planning/planning/src/chronicles/analysis/static_fluents.rs index fa7a86ff..00bd50cd 100644 --- a/planning/planning/src/chronicles/analysis/static_fluents.rs +++ b/planning/planning/src/chronicles/analysis/static_fluents.rs @@ -61,14 +61,13 @@ pub fn is_static(target_fluent: &Fluent, pb: &Problem) -> bool { .map(|tempplate| &tempplate.chronicle) .chain(pb.chronicles.iter().map(|ch| &ch.chronicle)); let mut conditions = chronicles.flat_map(|ch| ch.conditions.iter()); - let conditions_ok = conditions.all(|cond| { + + conditions.all(|cond| { if is_on_target_fluent(&cond.state_var) { // the value of this condition must be transformable to an int cond.value.int_view().is_some() } else { true // not interesting, continue } - }); - - conditions_ok + }) } diff --git a/planning/planning/src/chronicles/preprocessing/action_rolling.rs b/planning/planning/src/chronicles/preprocessing/action_rolling.rs index 84df4968..9ba7172e 100644 --- a/planning/planning/src/chronicles/preprocessing/action_rolling.rs +++ b/planning/planning/src/chronicles/preprocessing/action_rolling.rs @@ -349,7 +349,7 @@ impl Graph { /// Computes all shortest-path from this node pub fn shortest_paths(&self, from: Node) -> impl Iterator + '_ { - pathfinding::directed::dijkstra::dijkstra_reach(&from, |n, _c| self.adjacency[&n].iter().copied()) + pathfinding::directed::dijkstra::dijkstra_reach(&from, |n, _c| self.adjacency[n].iter().copied()) .map(|item| (item.node, item.total_cost)) } @@ -357,7 +357,7 @@ impl Graph { pub fn shortest_path(&self, from: Node, to: Node) -> Vec<(Node, Node, Cost)> { let (parents, res) = pathfinding::directed::dijkstra::dijkstra_partial( &from, - |n| self.adjacency[&n].iter().copied(), + |n| self.adjacency[n].iter().copied(), |cur| cur == &to, ); assert_eq!(res, Some(to)); diff --git a/solver/src/collections/ref_store.rs b/solver/src/collections/ref_store.rs index 3a54e48d..4907ca62 100644 --- a/solver/src/collections/ref_store.rs +++ b/solver/src/collections/ref_store.rs @@ -142,9 +142,9 @@ where &self.internal[k.into()] } - pub fn get_ref(&self, v: &W) -> Option + pub fn get_ref(&self, v: &W) -> Option where - W: Eq + Hash, + W: Eq + Hash + ?Sized, V: Eq + Hash + Borrow, { self.rev.get(v).copied() diff --git a/solver/src/model/model_impl.rs b/solver/src/model/model_impl.rs index a2b97f08..16c13800 100644 --- a/solver/src/model/model_impl.rs +++ b/solver/src/model/model_impl.rs @@ -411,7 +411,6 @@ impl Model { *expr = ReifExpr::Lit(Lit::FALSE); } else if lb1 == ub1 && ub1 == lb2 && lb2 == ub2 { *expr = ReifExpr::Lit(Lit::TRUE); - } else { } } ReifExpr::EqVal(v1, v2) => { diff --git a/solver/src/model/symbols.rs b/solver/src/model/symbols.rs index 8a55462c..4be7af77 100644 --- a/solver/src/model/symbols.rs +++ b/solver/src/model/symbols.rs @@ -153,9 +153,9 @@ impl SymbolTable { /// Retrieves the ID of a given symbol. Return None if the symbol doesn't appear in the /// symbol table. - pub fn id(&self, sym: &W) -> Option + pub fn id(&self, sym: &W) -> Option where - W: Eq + Hash, + W: Eq + Hash + ?Sized, Sym: Eq + Hash + Borrow, { self.ids.get(sym).copied() diff --git a/solver/src/model/types.rs b/solver/src/model/types.rs index 92053d08..4e3d59d3 100644 --- a/solver/src/model/types.rs +++ b/solver/src/model/types.rs @@ -115,9 +115,9 @@ impl TypeHierarchy { self.id_of(&self.top_type).unwrap() } - pub fn id_of(&self, tpe: &T2) -> Option + pub fn id_of(&self, tpe: &T2) -> Option where - T2: Eq + Hash, + T2: Eq + Hash + ?Sized, Sym: Eq + Hash + Borrow, { self.types.get_ref(tpe) diff --git a/solver/src/reasoners/eq/dense.rs b/solver/src/reasoners/eq/dense.rs index 6ded143a..d4adcb48 100644 --- a/solver/src/reasoners/eq/dense.rs +++ b/solver/src/reasoners/eq/dense.rs @@ -89,6 +89,7 @@ struct DirEdgeLabel { } #[derive(Clone, Debug)] +#[allow(clippy::enum_variant_names)] enum Event { EdgePropagation { x: Node, y: Node, z: Node }, EdgeEnabledPos(DirEdge), @@ -106,6 +107,7 @@ enum InferenceCause { } impl From for u32 { + #[allow(clippy::identity_op)] fn from(value: InferenceCause) -> Self { use InferenceCause::*; match value { @@ -274,8 +276,8 @@ impl DenseEqTheory { active, }; - self.graph.watches.add_watch(de.clone(), label); - self.graph.watches.add_watch(de.clone(), !label); + self.graph.watches.add_watch(de, label); + self.graph.watches.add_watch(de, !label); self.graph.watches.add_watch(de, active); self.graph .labels @@ -333,14 +335,8 @@ impl DenseEqTheory { if self.graph.enabled.contains(&e.id()) { return false; // edge is already enabled } - debug_assert!(self.graph.succs_pos[&e.src] - .iter() - .find(|ee| ee.succ == e.tgt) - .is_none()); - debug_assert!(self.graph.succs_neg[&e.src] - .iter() - .find(|ee| ee.succ == e.tgt) - .is_none()); + debug_assert!(!self.graph.succs_pos[&e.src].iter().any(|ee| ee.succ == e.tgt)); + debug_assert!(!self.graph.succs_neg[&e.src].iter().any(|ee| ee.succ == e.tgt)); match domains.value(e.label) { // enable the positive edge @@ -380,6 +376,7 @@ impl DenseEqTheory { } /// This edge was just enabled, propagate it + #[allow(clippy::option_map_unit_fn)] fn propagate_new_edge(&mut self, e: DirEdge, domains: &mut Domains) -> Result<(), InvalidUpdate> { let mut in_to_check: Vec = Vec::with_capacity(64); let mut out_to_check: Vec = Vec::with_capacity(64); @@ -900,6 +897,13 @@ impl ReifyEq for Model { fn domains(&self) -> &Domains { &self.state } + fn domain(&self, a: Node) -> (IntCst, IntCst) { + match a { + Node::Var(v) => self.state.bounds(v), + Node::Val(v) => (v, v), + } + } + fn reify_eq(&mut self, a: Node, b: Node) -> Lit { use Node::*; match (a, b) { @@ -930,13 +934,6 @@ impl ReifyEq for Model { pb } } - - fn domain(&self, a: Node) -> (IntCst, IntCst) { - match a { - Node::Var(v) => self.state.bounds(v), - Node::Val(v) => (v, v), - } - } } #[cfg(test)] diff --git a/solver/src/reasoners/eq/domain.rs b/solver/src/reasoners/eq/domain.rs index 96b4ad70..e8c5a74b 100644 --- a/solver/src/reasoners/eq/domain.rs +++ b/solver/src/reasoners/eq/domain.rs @@ -79,7 +79,7 @@ impl Domains { pub fn add_value(&mut self, var: VarRef, value: IntCst, lit: Lit) { self.domains .entry(var) - .or_insert_with(|| Domain::new()) + .or_insert_with(Domain::new) .add_value(value, lit); self.eq_watches.add_watch((var, value), lit); self.neq_watches.add_watch((var, value), !lit); diff --git a/solver/src/reasoners/eq/split.rs b/solver/src/reasoners/eq/split.rs index 06d47c9d..9dc4bfe3 100644 --- a/solver/src/reasoners/eq/split.rs +++ b/solver/src/reasoners/eq/split.rs @@ -68,7 +68,7 @@ impl SplitEqTheory { // they are in two distinct group, we need to merge them let (first, second) = if pa < pb { (pa, pb) } else { (pb, pa) }; debug_assert!(self.parts[second as usize].is_some()); - let to_delete = std::mem::replace(&mut self.parts[second as usize], None).unwrap(); + let to_delete = self.parts[second as usize].take().unwrap(); let final_group = self.parts[first as usize].as_mut().unwrap(); for var in to_delete.variables() { final_group.add_node(var, model); diff --git a/validator/src/interfaces/unified_planning/expression.rs b/validator/src/interfaces/unified_planning/expression.rs index af08e53b..dd757673 100644 --- a/validator/src/interfaces/unified_planning/expression.rs +++ b/validator/src/interfaces/unified_planning/expression.rs @@ -128,7 +128,7 @@ impl Interpreter for Expression { /* ========================= Utils Functions ======================== */ /// Checks that the number of arguments is correct for the procedure. - fn check_args(args: &Vec, expected: usize, proc_name: &String) -> Result<()> { + fn check_args(args: &[Expression], expected: usize, proc_name: &String) -> Result<()> { ensure!( args.len() == expected, format!( diff --git a/validator/src/interfaces/unified_planning/mod.rs b/validator/src/interfaces/unified_planning/mod.rs index 7841b78a..3e56919f 100644 --- a/validator/src/interfaces/unified_planning/mod.rs +++ b/validator/src/interfaces/unified_planning/mod.rs @@ -205,10 +205,7 @@ fn build_env(problem: &Problem, plan: &Plan, verbose: bool) -> Result, - parameters: &Vec, -) -> Result>> { +fn generate_fluent_parameters_combinations(env: &Env, parameters: &[upParam]) -> Result>> { if parameters.is_empty() { return Ok(vec![vec![]]); } @@ -221,7 +218,7 @@ fn generate_fluent_parameters_combinations( .context(format!("No objects of type {}", first_param.r#type))?; for obj in objects { - let next_combinations = generate_fluent_parameters_combinations(env, &next_params.to_vec())?; + let next_combinations = generate_fluent_parameters_combinations(env, next_params)?; for next_parameters in next_combinations { let mut combination = vec![obj.clone()]; combination.extend(next_parameters);