chore(ci): Unify compilation/execution report jobs that take averages…

… with single runs (noir-lang/noir#7048) fix(nargo_fmt): let doc comment could come after regular comment (noir-lang/noir#7046) fix(nargo_fmt): don't consider identifiers the same if they are equal… (noir-lang/noir#7043) feat: auto-import traits when suggesting trait methods (noir-lang/noir#7037) feat: avoid inserting `inc_rc` instructions into ACIR (noir-lang/noir#7036) fix(lsp): suggest all possible trait methods, but only visible ones (noir-lang/noir#7027) chore: add more protocol circuits to reports (noir-lang/noir#6977) feat: avoid generating a new witness when checking if linear expression is zero (noir-lang/noir#7031) feat: skip codegen of zero iteration loops (noir-lang/noir#7030)
AztecProtocol · Jan 14, 2025 · d5a650d · d5a650d
2 parents 1d11890 + a984346
commit d5a650d
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 35 deletions.
diff --git a/.noir-sync-commit b/.noir-sync-commit
@@ -1 +1 @@
-a9acf5a2fa8a673074e623e3ebd899bd24598649
+1e64f8a7e0481f43f8afeb3dc3c33300f6fa5857
diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml
@@ -274,15 +274,15 @@ jobs:
         run: |
           ./execution_report.sh 0 1
           mv execution_report.json ../execution_report.json
-
+          
       - name: Upload compilation report
         uses: actions/upload-artifact@v4
         with:
           name: in_progress_compilation_report
           path: compilation_report.json
           retention-days: 3
           overwrite: true
-
+      
       - name: Upload execution report
         uses: actions/upload-artifact@v4
         with:
@@ -300,17 +300,17 @@ jobs:
       matrix:
         include:
           # - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, take_average: true  }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, take_average: true }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, num_runs: 5 }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, num_runs: 5  }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, num_runs: 5 }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-private, num_runs: 5 }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-base-public, num_runs: 5 }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-merge, num_runs: 5 }
           #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root-empty }
           #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-single-tx }
           #- project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-block-root }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-merge, take_average: true }
-          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, take_average: true }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-merge, num_runs: 5 }
+          - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-root, num_runs: 5 }
 
     name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }}
     steps:
@@ -352,7 +352,15 @@ jobs:
           touch parse_time.sh
           chmod +x parse_time.sh
           cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
-          ./compilation_report.sh 1
+          ./compilation_report.sh 1 ${{ matrix.project.num_runs }}
+      
+      - name: Generate execution report
+        working-directory: ./test-repo/${{ matrix.project.path }}
+        if: ${{ !matrix.project.is_library }}
+        run: |
+          mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh
+          mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh
+          ./execution_report.sh 1 ${{ matrix.project.num_runs }}
 
       - name: Generate compilation report with averages
         working-directory: ./test-repo/${{ matrix.project.path }}
@@ -398,7 +406,7 @@ jobs:
           PACKAGE_NAME=$(basename $PACKAGE_NAME)
           mv ./test-repo/${{ matrix.project.path }}/execution_report.json ./execution_report_$PACKAGE_NAME.json
           echo "execution_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT
-
+          
       - name: Upload compilation report
         uses: actions/upload-artifact@v4
         with:
@@ -416,10 +424,10 @@ jobs:
           overwrite: true
 
   upload_compilation_report:
-    name: Upload compilation report
+    name: Upload compilation report 
     needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report]
     # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails
-    if: always()
+    if: always() 
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write
@@ -553,7 +561,7 @@ jobs:
           overwrite: true
 
   upload_compilation_memory_report:
-    name: Upload compilation memory report
+    name: Upload compilation memory report 
     needs: [generate_memory_report, external_repo_memory_report]
     # We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
     if: always()
@@ -599,10 +607,10 @@ jobs:
           message: ${{ steps.compilation_mem_report.outputs.markdown }}
 
   upload_execution_memory_report:
-    name: Upload execution memory report
+    name: Upload execution memory report 
     needs: [generate_memory_report, external_repo_memory_report]
     # We want this job to run even if one variation of the matrix in `external_repo_memory_report` fails
-    if: always()
+    if: always() 
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write
@@ -645,10 +653,10 @@ jobs:
           message: ${{ steps.execution_mem_report.outputs.markdown }}
 
   upload_execution_report:
-    name: Upload execution report
+    name: Upload execution report 
     needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report]
     # We want this job to run even if one variation of the matrix in `external_repo_compilation_and_execution_report` fails
-    if: always()
+    if: always() 
     runs-on: ubuntu-latest
     permissions:
       pull-requests: write

diff --git a/noir/noir-repo/noir_stdlib/src/collections/umap.nr b/noir/noir-repo/noir_stdlib/src/collections/umap.nr
@@ -114,8 +114,7 @@ impl<K, V, B> UHashMap<K, V, B> {
     {
         // docs:end:contains_key
         /// Safety: unconstrained context
-        unsafe { self.get(key) }
-            .is_some()
+        unsafe { self.get(key) }.is_some()
     }
 
     // Returns true if the map contains no elements.

diff --git a/noir/noir-repo/test_programs/compilation_report.sh b/noir/noir-repo/test_programs/compilation_report.sh
@@ -37,15 +37,9 @@ for dir in ${tests_to_profile[@]}; do
       PACKAGE_NAME=$(basename $current_dir)
     fi
 
-    NUM_RUNS=1
+    NUM_RUNS=$2
     TOTAL_TIME=0
 
-    # Passing a second argument will take an average of five runs
-    # rather than 
-    if [ "$2" == "1" ]; then
-      NUM_RUNS=5
-    fi
-
     for ((i = 1; i <= NUM_RUNS; i++)); do
       NOIR_LOG=trace NARGO_LOG_DIR=./tmp nargo compile --force --silence-warnings
     done

diff --git a/noir/noir-repo/test_programs/execution_report.sh b/noir/noir-repo/test_programs/execution_report.sh
@@ -46,12 +46,8 @@ for dir in ${tests_to_profile[@]}; do
     fi
 
 
-    NUM_RUNS=1
+    NUM_RUNS=$2
     TOTAL_TIME=0
-
-    if [ "$2" == "1" ]; then
-      NUM_RUNS=5
-    fi
 
     for ((i = 1; i <= NUM_RUNS; i++)); do
       NOIR_LOG=trace NARGO_LOG_DIR=./tmp nargo execute --silence-warnings

diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/statement.rs
@@ -23,10 +23,17 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
 
         // Now write any leading comment respecting multiple newlines after them
         group.leading_comment(self.chunk(|formatter| {
+            // Doc comments for a let statement could come before a potential non-doc comment
             if formatter.token.kind() == TokenKind::OuterDocComment {
                 formatter.format_outer_doc_comments();
             }
+
             formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found();
+
+            // Or doc comments could come after a potential non-doc comment
+            if formatter.token.kind() == TokenKind::OuterDocComment {
+                formatter.format_outer_doc_comments();
+            }
         }));
 
         ignore_next |= self.ignore_next;
@@ -390,6 +397,21 @@ mod tests {
         assert_format(src, expected);
     }
 
+    #[test]
+    fn format_let_statement_with_unsafe_and_comment_before_it() {
+        let src = " fn foo() { 
+        // Some comment
+        /// Safety: some doc
+        let  x  =  unsafe { 1 } ; } ";
+        let expected = "fn foo() {
+    // Some comment
+    /// Safety: some doc
+    let x = unsafe { 1 };
+}
+";
+        assert_format(src, expected);
+    }
+
     #[test]
     fn format_assign() {
         let src = " fn foo() { x  =  2 ; } ";

diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/use_tree_merge.rs
@@ -179,7 +179,12 @@ impl Ord for Segment {
 
         if let (Segment::Plain(self_string), Segment::Plain(other_string)) = (self, other) {
             // Case-insensitive comparison for plain segments
-            self_string.to_lowercase().cmp(&other_string.to_lowercase())
+            let ordering = self_string.to_lowercase().cmp(&other_string.to_lowercase());
+            if ordering == Ordering::Equal {
+                self_string.cmp(other_string)
+            } else {
+                ordering
+            }
         } else {
             order_number_ordering
         }
@@ -620,4 +625,10 @@ use std::merkle::compute_merkle_root;
         let expected = "use std::{as_witness, merkle::compute_merkle_root};\n";
         assert_format(src, expected);
     }
+
+    #[test]
+    fn does_not_merge_same_identifiers_if_equal_case_insensitive() {
+        let src = "use bigint::{BigNum, bignum::BigNumTrait};\n";
+        assert_format(src, src);
+    }
 }
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		a9acf5a2fa8a673074e623e3ebd899bd24598649
		1e64f8a7e0481f43f8afeb3dc3c33300f6fa5857