From 242f438cf2fa4bd65d2110f0469daf10afea867a Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 13 Sep 2023 16:48:22 +0000 Subject: [PATCH 1/8] chore(ci): treat clippy warnings as errors --- .github/workflows/formatting.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 8d29886e40c..4b55e10f353 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -44,6 +44,8 @@ jobs: - name: Run `cargo clippy` run: cargo clippy --workspace --locked --release + env: + RUSTFLAGS: -Dwarnings - name: Run `cargo fmt` run: cargo fmt --all --check From 4ac729bc6ddaa9766bdc3a49de687d83be4d6359 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 13 Sep 2023 16:48:43 +0000 Subject: [PATCH 2/8] chore: address clippy warning --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index a2943596a53..870aeb9e559 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -556,14 +556,14 @@ impl Context { let predicate_index = self.acir_context.mul_var(index_var, self.current_side_effects_enabled_var)?; let new_value = if let Some(store) = store_value { - let store_var = Some(self.convert_value(store, dfg).into_var()?); + let store_var = self.convert_value(store, dfg).into_var()?; if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) { - store_var + Some(store_var) } else { let dummy = self.array_get(instruction, array, predicate_index, dfg)?; let true_pred = self .acir_context - .mul_var(store_var.unwrap(), self.current_side_effects_enabled_var)?; + .mul_var(store_var, self.current_side_effects_enabled_var)?; let one = self.acir_context.add_constant(FieldElement::one()); let not_pred = self.acir_context.sub_var(one, self.current_side_effects_enabled_var)?; From 12dd7bbfa64319628ef4dd1477a59d2e0a583ad0 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 13 Sep 2023 16:53:49 +0000 Subject: [PATCH 3/8] chore: take RUSTFLAGS into account when caching --- .github/workflows/formatting.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 4b55e10f353..101df6cff88 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -17,7 +17,9 @@ jobs: name: cargo clippy runs-on: ${{ matrix.runner }} timeout-minutes: 30 - + env: + RUSTFLAGS: -Dwarnings + strategy: fail-fast: false matrix: @@ -44,8 +46,6 @@ jobs: - name: Run `cargo clippy` run: cargo clippy --workspace --locked --release - env: - RUSTFLAGS: -Dwarnings - name: Run `cargo fmt` run: cargo fmt --all --check From 8f0996b5c0d77e3cf1432459b550b9e6f8e9ce5f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 13 Sep 2023 16:55:00 +0000 Subject: [PATCH 4/8] chore: run `cargo fmt` --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 870aeb9e559..1a58386c139 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -561,9 +561,8 @@ impl Context { Some(store_var) } else { let dummy = self.array_get(instruction, array, predicate_index, dfg)?; - let true_pred = self - .acir_context - .mul_var(store_var, self.current_side_effects_enabled_var)?; + let true_pred = + self.acir_context.mul_var(store_var, self.current_side_effects_enabled_var)?; let one = self.acir_context.add_constant(FieldElement::one()); let not_pred = self.acir_context.sub_var(one, self.current_side_effects_enabled_var)?; From b431985207c40bbe07dc88e8eb470d2eca97eb8c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 13 Sep 2023 17:19:51 +0000 Subject: [PATCH 5/8] chore: address clippy errors from nightly --- compiler/noirc_evaluator/src/ssa/ir/map.rs | 2 +- compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs | 2 +- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 4 +--- compiler/noirc_frontend/src/ast/statement.rs | 2 +- compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 2 +- compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs | 2 +- compiler/noirc_frontend/src/hir/def_map/mod.rs | 4 ++-- compiler/noirc_frontend/src/hir/type_check/mod.rs | 5 +---- 8 files changed, 9 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/map.rs b/compiler/noirc_evaluator/src/ssa/ir/map.rs index 10d6adfbd6a..b6055973f1c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/map.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/map.rs @@ -52,7 +52,7 @@ impl std::hash::Hash for Id { impl PartialOrd for Id { fn partial_cmp(&self, other: &Self) -> Option { - self.index.partial_cmp(&other.index) + Some(self.cmp(other)) } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 9eda52e0475..e71e23d2032 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -125,7 +125,7 @@ impl Context { fn resolve_instruction( instruction_id: InstructionId, dfg: &DataFlowGraph, - constrained_values: &mut HashMap, + constrained_values: &HashMap, ) -> Instruction { let instruction = dfg[instruction_id].clone(); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 652869bdc9d..20250f5470e 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -527,9 +527,7 @@ impl<'a> FunctionContext<'a> { ) -> Values { match expr { // If we're constraining an equality to be true then constrain the two sides directly. - Expression::Binary(Binary { lhs, operator, rhs, .. }) - if operator == &BinaryOpKind::Equal => - { + Expression::Binary(Binary { lhs, operator: BinaryOpKind::Equal, rhs, .. }) => { let lhs = self.codegen_non_tuple_expression(lhs); let rhs = self.codegen_non_tuple_expression(rhs); self.builder.set_location(location).insert_constrain(lhs, rhs, assert_message); diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 51afa688082..3d9ab1e6ec4 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -125,7 +125,7 @@ impl PartialEq for Ident { impl PartialOrd for Ident { fn partial_cmp(&self, other: &Self) -> Option { - self.0.contents.partial_cmp(&other.0.contents) + Some(self.cmp(other)) } } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index c55335e4443..11321d673a7 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -530,13 +530,13 @@ fn take_errors_filter_self_not_resolved(resolver: Resolver<'_>) -> Vec { &ident.0.contents != "Self" } _ => true, }) + .cloned() .collect() } diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 2679059cebd..813c222319e 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -470,7 +470,7 @@ impl<'a> ModCollector<'a> { }; // Parse the AST for the module we just found and then recursively look for it's defs - let ast = parse_file(&mut context.file_manager, child_file_id, errors); + let ast = parse_file(&context.file_manager, child_file_id, errors); // Add module into def collector and get a ModuleId if let Some(child_mod_id) = diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 4c7f241efaa..9a55fc3afd1 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -86,7 +86,7 @@ impl CrateDefMap { // First parse the root file. let root_file_id = context.crate_graph[crate_id].root_file_id; - let ast = parse_file(&mut context.file_manager, root_file_id, errors); + let ast = parse_file(&context.file_manager, root_file_id, errors); #[cfg(feature = "aztec")] let ast = aztec_library::transform(ast, &crate_id, context, errors); @@ -214,7 +214,7 @@ pub struct Contract { /// Given a FileId, fetch the File, from the FileManager and parse it's content pub fn parse_file( - fm: &mut FileManager, + fm: &FileManager, file_id: FileId, all_errors: &mut Vec, ) -> ParsedModule { diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index ea1793b7e76..a2f9361ad3a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -90,10 +90,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec (noirc_errors::Span, bool) { +fn function_info(interner: &NodeInterner, function_body_id: &ExprId) -> (noirc_errors::Span, bool) { let (expr_span, empty_function) = if let HirExpression::Block(block) = interner.expression(function_body_id) { let last_stmt = block.statements().last(); From ac384b0883f0f1cdd596ebe0094e3229799125b5 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 15 Sep 2023 18:14:34 +0100 Subject: [PATCH 6/8] chore(ci): lock clippy to 1.66.0 --- .github/workflows/formatting.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/formatting.yml b/.github/workflows/formatting.yml index 101df6cff88..bdc95b40bd5 100644 --- a/.github/workflows/formatting.yml +++ b/.github/workflows/formatting.yml @@ -32,9 +32,8 @@ jobs: uses: actions/checkout@v4 - name: Setup toolchain - uses: dtolnay/rust-toolchain@master + uses: dtolnay/rust-toolchain@1.66.0 with: - toolchain: stable # We do not use MSRV so we can benefit from newer lints targets: ${{ matrix.target }} components: clippy, rustfmt From f00330c825cab5eca53f76739ef37e521573e411 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 15 Sep 2023 18:27:48 +0100 Subject: [PATCH 7/8] chore: fix clippy errors --- compiler/noirc_frontend/src/hir/def_map/mod.rs | 18 +++++++++--------- .../noirc_frontend/src/hir/type_check/mod.rs | 3 ++- compiler/noirc_frontend/src/node_interner.rs | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 576a348383a..bc6e413cd3d 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -160,15 +160,15 @@ impl CrateDefMap { .iter() .filter_map(|(id, module)| { if module.is_contract { - let function_ids: Vec = - module.value_definitions().filter_map(|id| id.as_function()).collect(); - - let functions = function_ids - .into_iter() - .map(|id| { - let is_entry_point = - !interner.function_attributes(&id).has_contract_library_method(); - ContractFunctionMeta { function_id: id, is_entry_point } + let functions: Vec = module + .value_definitions() + .filter_map(|id| { + id.as_function().map(|function_id| { + let is_entry_point = !interner + .function_attributes(&function_id) + .has_contract_library_method(); + ContractFunctionMeta { function_id, is_entry_point } + }) }) .collect(); diff --git a/compiler/noirc_frontend/src/hir/type_check/mod.rs b/compiler/noirc_frontend/src/hir/type_check/mod.rs index 9165a308007..d690fe2458a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/mod.rs +++ b/compiler/noirc_frontend/src/hir/type_check/mod.rs @@ -397,8 +397,9 @@ mod test { } fn type_check_src_code(src: &str, func_namespace: Vec) { - type_check_src_code_errors_expected(src, 0, func_namespace) + type_check_src_code_errors_expected(src, 0, func_namespace); } + // This function assumes that there is only one function and this is the // func id that is returned fn type_check_src_code_errors_expected( diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index cffa5e92e3f..e0e404b48d8 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -549,7 +549,7 @@ impl NodeInterner { } pub fn function_attributes(&self, func_id: &FuncId) -> Attributes { - self.function_meta(func_id).attributes.clone() + self.function_meta(func_id).attributes } /// Returns the interned statement corresponding to `stmt_id` From 9601bcae69e99d0e7ff81808385e3a601c5e6255 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 15 Sep 2023 18:28:38 +0100 Subject: [PATCH 8/8] chore: remove unnecessary type --- compiler/noirc_frontend/src/hir/def_map/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index bc6e413cd3d..76db8ca753c 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -160,7 +160,7 @@ impl CrateDefMap { .iter() .filter_map(|(id, module)| { if module.is_contract { - let functions: Vec = module + let functions = module .value_definitions() .filter_map(|id| { id.as_function().map(|function_id| {