[CI] Standardize a single rust toolchain for all of CI (#2374)

Supersedes #2369.

`rust-toolchain.toml` now determines the version of rust used for every
aspect of the CI, rather than having it change on its own or be set
separately. As it turns out, the version of rust used for the clippy
lints was also pinned separately, so I also had to address all those
lints. As a result, I had to make minor changes to a bunch of code, the
vast majority of which were either docstring indents or places where
people manually implemented `ToString` instead of using `Display`. I
don't anticipate any of these changes messing with things but if I did
break something, do let me know.

Assuming I've done things correctly, it should be the case that the rust
version will not change on its own anymore, and we can update the
version used by all of Ci by editing the toolchain file

.github/workflows/docs.yml

@@ -23,10 +23,8 @@ jobs:
       - name: mdbook
         run: mdbook build
       - name: Install Rust stable
-        uses: actions-rs/toolchain@v1
+        uses: actions-rust-lang/setup-rust-toolchain@v1
         with:
-          toolchain: 1.66.0
-          override: true
           components: rustfmt, clippy
       - name: Build source documentation
         uses: actions-rs/cargo@v1

.github/workflows/format.yml

@@ -13,10 +13,8 @@ jobs:
     - name: Checkout
       uses: actions/checkout@v4
     - name: Install stable
-      uses: actions-rs/toolchain@v1
+      uses: actions-rust-lang/setup-rust-toolchain@v1
       with:
-          toolchain: 1.76.0
-          override: true
           components: rustfmt, clippy
     - name: Check formatting
       uses: actions-rs/cargo@v1

.github/workflows/playground.yml

@@ -14,8 +14,6 @@ jobs:
       - uses: actions/setup-node@v4
       - name: Install Rust stable
         uses: actions-rust-lang/setup-rust-toolchain@v1
-        with:
-            toolchain: stable
       - name: Install wasm-pack and wasm-bindgen-cli
         uses: taiki-e/install-action@wasm-pack
         with:

calyx-backend/src/backend_opt.rs

@@ -62,8 +62,8 @@ impl FromStr for BackendOpt {
 }
 
 /// Convert `BackendOpt` to a string
-impl ToString for BackendOpt {
-    fn to_string(&self) -> String {
+impl std::fmt::Display for BackendOpt {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::Mlir => "mlir",
             Self::Resources => "resources",
@@ -76,6 +76,6 @@ impl ToString for BackendOpt {
             Self::PrimitiveUses => "primitive-uses",
             Self::None => "none",
         }
-        .to_string()
+        .fmt(f)
     }
 }

calyx-backend/src/verilog.rs

@@ -419,7 +419,7 @@ fn cell_instance(cell: &ir::Cell) -> Option<v::Instance> {
                     );
                 } else {
                     param_binding.iter().for_each(|(name, value)| {
-                    if *value > (std::i32::MAX as u64) {
+                    if *value > (i32::MAX as u64) {
                         panic!(
                             "Parameter value {} for `{}` cannot be represented using 32 bits",
                             value,

calyx-ir/src/guard.rs

@@ -2,7 +2,7 @@ use crate::Printer;
 
 use super::{NumAttr, Port, RRC};
 use calyx_utils::Error;
-use std::fmt::Debug;
+use std::fmt::{Debug, Display};
 use std::mem;
 use std::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, Not};
 use std::{cmp::Ordering, hash::Hash, rc::Rc};
@@ -11,9 +11,9 @@ use std::{cmp::Ordering, hash::Hash, rc::Rc};
 #[cfg_attr(feature = "serialize", derive(serde::Serialize))]
 pub struct Nothing;
 
-impl ToString for Nothing {
-    fn to_string(&self) -> String {
-        "".to_string()
+impl Display for Nothing {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "")
     }
 }
 
@@ -62,12 +62,12 @@ pub struct StaticTiming {
     interval: (u64, u64),
 }
 
-impl ToString for StaticTiming {
-    fn to_string(&self) -> String {
+impl Display for StaticTiming {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if self.interval.0 + 1 == self.interval.1 {
-            format!("%{}", self.interval.0)
+            write!(f, "%{}", self.interval.0)
         } else {
-            format!("%[{}:{}]", self.interval.0, self.interval.1)
+            write!(f, "%[{}:{}]", self.interval.0, self.interval.1)
         }
     }
 }

calyx-ir/src/printer.rs

@@ -759,9 +759,9 @@ impl Printer {
     }
 
     /// Generate a String-based representation for a guard.
-    pub fn guard_str<T: ToString>(guard: &ir::Guard<T>) -> String
+    pub fn guard_str<T>(guard: &ir::Guard<T>) -> String
     where
-        T: Eq,
+        T: Eq + ToString,
     {
         match &guard {
             ir::Guard::And(l, r) | ir::Guard::Or(l, r) => {

calyx-opt/src/analysis/domination_analysis/dominator_map.rs

@@ -23,21 +23,22 @@ const END_ID: ir::Attribute = ir::Attribute::Internal(ir::InternalAttr::END_ID);
 /// - While Guards
 /// - If Guards
 /// - "End" If nodes, representing the place we're at in the program after the if
-/// statement has just finished. This doesn't correspond to any actual Calyx code, but is
-/// just a conceptualization we use to reason about domination.
+///   statement has just finished. This doesn't correspond to any actual Calyx code, but is
+///   just a conceptualization we use to reason about domination.
+///
 /// Note that seqs and pars will *not* be included in the domination map.
 ///
 /// Here is the algorithm we use to build the domination map.
 /// - Start with an emtpy map.
 /// - Visit each node n in the control program, and set:
 /// - dom(n) = {U dom(p) for each predecessor p of n} U {n}. In other words, take the
-/// dominators of each predecessor of n, and union them together. Then add n to
-/// this set, and set this set as the dominators of n.
+///   dominators of each predecessor of n, and union them together. Then add n to
+///   this set, and set this set as the dominators of n.
 /// - (Another clarification): by "predecessors" of node n we mean the set of nodes
-/// that could be the most recent node executed when n begins to execute.
+///   that could be the most recent node executed when n begins to execute.
 /// - If we visit every node of the control program and the map has not changed,
-/// then we are done. If it has changed, then we visit each node again to repeat
-/// the process.
+///   then we are done. If it has changed, then we visit each node again to repeat
+///   the process.
 ///
 /// The reason why we can take the union (rather than intersection) of the
 /// dominators of each predecessor is because we know each predecessor of each
@@ -46,19 +47,19 @@ const END_ID: ir::Attribute = ir::Attribute::Internal(ir::InternalAttr::END_ID);
 /// our algorithm to deal with them.
 ///
 /// 1) The While Guard
-/// The last node(s) in the while body are predecessor(s) of the while guard but
-/// are not guaranteed to be executed. So, we can think of the while guard's
-/// predecessors as being split in two groups: the "body predecessors" that are not guaranteed to
-/// be executed before the while guard and the "outside predecessors" that are
-/// outside the body of the while loop and are guaranteed to be executed before
-/// the while loop guard.
-/// Here we take:
-/// dom(while guard) = U(dom(outside preds)) U {while guard}
+///    The last node(s) in the while body are predecessor(s) of the while guard but
+///    are not guaranteed to be executed. So, we can think of the while guard's
+///    predecessors as being split in two groups: the "body predecessors" that are not guaranteed to
+///    be executed before the while guard and the "outside predecessors" that are
+///    outside the body of the while loop and are guaranteed to be executed before
+///    the while loop guard.
+///    Here we take:
+///    dom(while guard) = U(dom(outside preds)) U {while guard}
 ///
 /// Justification:
 /// dom(while guard) is a subset of U(dom(outside preds)) U {while guard}
 /// Suppose n dominates the while guard. Every path to the while guard must end in
-/// 1) outside pred -> while guard OR 2) body pred -> while guard. But for choice 2)
+/// (1) outside pred -> while guard OR (2) body pred -> while guard. But for choice 2)
 /// we know the path was really something like outside pred -> while guard -> body
 /// -> while guard... body -> while guard. Since n dominates the while guard
 /// we know that it *cannot* be in the while body. Therefore, since every path to the
@@ -72,10 +73,10 @@ const END_ID: ir::Attribute = ir::Attribute::Internal(ir::InternalAttr::END_ID);
 /// n dominates the while guard.
 ///
 /// 2) "End Node" of If Statements
-/// In this case, *neither* of the predecessor sets (the set in the tbranch or
-/// the set in the fbranch) are guaranteed to be executed.
-/// Here we take:
-/// dom(end node) = dom(if guard) U {end node}.
+///    In this case, *neither* of the predecessor sets (the set in the tbranch or
+///    the set in the fbranch) are guaranteed to be executed.
+///    Here we take:
+///    dom(end node) = dom(if guard) U {end node}.
 ///
 /// Justification:
 /// dom(end node) is a subset of dom(if guard) U {end node}.

calyx-opt/src/analysis/graph.rs

@@ -5,7 +5,7 @@ use petgraph::{
     visit::EdgeRef,
     Direction::{Incoming, Outgoing},
 };
-use std::fmt::Write;
+use std::fmt::{Display, Write};
 use std::{collections::HashMap, rc::Rc};
 
 type Node = RRC<ir::Port>;
@@ -271,8 +271,8 @@ impl GraphAnalysis {
     }
 }
 
-impl ToString for GraphAnalysis {
-    fn to_string(&self) -> String {
+impl Display for GraphAnalysis {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let mut out = String::new();
         for idx in self.graph.node_indices() {
             let src_port = self.graph[idx].borrow();
@@ -293,6 +293,6 @@ impl ToString for GraphAnalysis {
             )
             .expect("Failed to write to ScheduleConflicts string");
         }
-        out
+        out.fmt(f)
     }
 }

calyx-opt/src/analysis/graph_coloring.rs

@@ -3,6 +3,7 @@ use itertools::Itertools;
 use petgraph::algo;
 use std::{
     collections::{BTreeMap, HashMap},
+    fmt::Display,
     hash::Hash,
 };
 
@@ -216,8 +217,8 @@ where
     }
 }
 
-impl<T: Eq + Hash + ToString + Clone + Ord> ToString for GraphColoring<T> {
-    fn to_string(&self) -> String {
-        self.graph.to_string()
+impl<T: Eq + Hash + ToString + Clone + Ord> Display for GraphColoring<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.graph.to_string().fmt(f)
     }
 }

calyx-opt/src/analysis/inference_analysis.rs

@@ -535,10 +535,11 @@ impl InferenceAnalysis {
 
     /// "Fixes Up" the component. In particular:
     /// 1. Removes @promotable annotations for any groups that write to any
-    /// `updated_components`.
+    ///    `updated_components`.
     /// 2. Try to re-infer groups' latencies.
     /// 3. Removes all @promotable annotation from the control program.
     /// 4. Re-infers the @promotable annotations for any groups or control.
+    ///
     /// Note that this only fixes up the component's ``internals''.
     /// It does *not* fix the component's signature.
     pub fn fixup_timing(&self, comp: &mut ir::Component) {

calyx-opt/src/analysis/static_tree.rs

@@ -367,13 +367,13 @@ impl SingleNode {
     /// Therefore we take in a bunch of data structures to keep track of coloring:
     ///   - `coloring` that maps group names -> colors,
     ///   - `colors_to_max_values` which maps colors -> (max latency, max_num_repeats)
-    ///   (we need to make sure that when we instantiate a color,
-    ///   we give enough bits to support the maximum latency/num_repeats that will be
-    ///   used for that color)
+    ///     (we need to make sure that when we instantiate a color,
+    ///     we give enough bits to support the maximum latency/num_repeats that will be
+    ///     used for that color)
     ///   - `colors_to_fsm`
-    ///   which maps colors to (fsm_register, iter_count_register): fsm_register counts
-    ///   up for a single iteration, iter_count_register counts the number of iterations
-    ///   that have passed.
+    ///     which maps colors to (fsm_register, iter_count_register): fsm_register counts
+    ///     up for a single iteration, iter_count_register counts the number of iterations
+    ///     that have passed.
     ///
     /// Note that it is not always necessary to instantiate one or both registers (e.g.,
     /// if num_repeats == 1 then you don't need an iter_count_register).
@@ -1279,6 +1279,7 @@ impl SingleNode {
     ///   - if `global_view` is true, then you have to include the iteration
     ///     count register in the assignment's guard.
     ///   - if `global_view` is false, then you dont' have to include it
+    ///
     /// `ignore_timing`: remove static timing guards instead of transforming them
     /// into an FSM query. Note that in order to do this, the timing guard must
     /// equal %[0:1], otherwise we will throw an error. This option is here

calyx-opt/src/analysis/variable_detection.rs

@@ -10,6 +10,7 @@ impl VariableDetection {
     ///  - among write to state_shareable components, there is only one write
     ///  - has `@go` port equal to `1'd1`
     ///  - has `g[done] = cell.done`
+    ///
     /// Returns the name of the cell if such a group is detected,
     /// otherwise returns `None`.
     pub fn variable_like(

calyx-opt/src/passes/cell_share.rs

@@ -282,12 +282,12 @@ impl CellShare {
 ///  - use [ScheduleConflicts] to find groups/invokes that run in parallel with each other
 ///  - for each tuple combination of cells that return true on cell_filter(), c1 and c2
 ///  - first determine if their live ranges overlap. If so, then insert a conflict between
-///  c1 and c2
+///    c1 and c2
 ///  - if c1 and c2 don't have overlapping live ranges, check if c1 and c2 are ever
-///  live at within the same par block, and they are live at different children
-///  of the par block. If the parent par is not static, then add a conflict.
-///  If the parent par is static, then we can use the static_par_timing analysis
-///  to check whether the cells' liveness actually overlaps.
+///    live at within the same par block, and they are live at different children
+///    of the par block. If the parent par is not static, then add a conflict.
+///    If the parent par is static, then we can use the static_par_timing analysis
+///    to check whether the cells' liveness actually overlaps.
 ///  - perform graph coloring using `self.ordering` to define the order of the greedy coloring
 ///  - use coloring to rewrite group assignments, continuous assignments, and conditional ports.
 impl Visitor for CellShare {

calyx-opt/src/passes/collapse_control.rs

@@ -36,7 +36,6 @@ use calyx_ir::{self as ir, GetAttributes, LibrarySignatures};
 /// 3. Collapses nested `static seq` in the same way as 1
 /// 4. Collapses nested `static par` in the same way as 2
 /// 5. Collapses `static repeat`:
-/// Collapse
 /// ```
 /// static repeat 0 { ** body ** }
 /// ```

calyx-opt/src/passes/group_to_invoke.rs

@@ -13,9 +13,9 @@ use std::rc::Rc;
 ///
 /// For a group to meet the requirements of this pass, it must
 /// 1. Only write to one non-combinational component (all other writes must be
-/// to combinational primitives)
+///    to combinational primitives)
 /// 2. That component is *not* a ref cell, nor does it have the external attribute,
-/// nor is it This Component
+///    nor is it This Component
 /// 3. Assign component.go = 1'd1
 /// 4. Assign group[done] = component.done
 pub struct GroupToInvoke {

calyx-opt/src/passes/lower_guards.rs

@@ -145,12 +145,11 @@ impl Visitor for LowerGuards {
             .component
             .get_groups_mut()
             .drain()
-            .map(|group| {
+            .inspect(|group| {
                 let assigns =
                     group.borrow_mut().assignments.drain(..).collect();
                 let new_assigns = lower_assigns(assigns, &mut builder);
                 group.borrow_mut().assignments = new_assigns;
-                group
             })
             .into();
         builder.component.set_groups(groups);
@@ -174,12 +173,11 @@ impl Visitor for LowerGuards {
             .component
             .comb_groups
             .drain()
-            .map(|group| {
+            .inspect(|group| {
                 let assigns =
                     group.borrow_mut().assignments.drain(..).collect();
                 let new_assigns = lower_assigns(assigns, &mut builder);
                 group.borrow_mut().assignments = new_assigns;
-                group
             })
             .into();
         builder.component.comb_groups = comb_groups;

calyx-opt/src/passes/static_promotion.rs

@@ -21,12 +21,12 @@ const APPROX_WHILE_REPEAT_SIZE: u64 = 3;
 ///
 /// Promotion occurs the following policies:
 /// 1. ``Threshold'': How large the island must be. We have three const
-/// defined as heuristics to measure approximately how big each control program
-/// is. It must be larger than that threshold.
+///    defined as heuristics to measure approximately how big each control program
+///    is. It must be larger than that threshold.
 /// 2. ``Cycle limit": The maximum number of cycles the island can be when we
-/// promote it.
+///    promote it.
 /// 3. ``If Diff Limit": The maximum difference in latency between if statments
-/// that we can tolerate to promote it.
+///    that we can tolerate to promote it.
 ///
 pub struct StaticPromotion {
     /// An InferenceAnalysis object so that we can re-infer the latencies of

calyx-opt/src/passes/well_formed.rs

@@ -541,13 +541,13 @@ impl Visitor for WellFormed {
                         {
                             Err(Error::malformed_structure(format!(
                                 "Static Timing Guard has improper interval: `{}`",
-                                static_timing.to_string()
+                                static_timing
                             ))
                             .with_pos(&assign.attributes))
                         } else if static_timing.get_interval().1 > group_latency {
                             Err(Error::malformed_structure(format!(
                                 "Static Timing Guard has interval `{}`, which is out of bounds since its static group has latency {}",
-                                static_timing.to_string(),
+                                static_timing,
                                 group_latency
                             ))
                             .with_pos(&assign.attributes))

calyx-opt/src/passes_experimental/sync/compile_sync_without_sync_reg.rs

@@ -64,7 +64,7 @@ impl BarrierMap {
     }
 
     fn insert_shared_wire(&mut self, builder: &mut ir::Builder, idx: &u64) {
-        if self.0.get(idx).is_none() {
+        if !self.0.contains_key(idx) {
             structure!(builder;
                 let s = prim std_wire(1);
             );

calyx-opt/src/traversal/construct.rs

@@ -106,7 +106,7 @@ impl std::fmt::Display for ParseVal {
                 }
                 write!(f, "]")
             }
-            ParseVal::OutStream(o) => write!(f, "{}", o.to_string()),
+            ParseVal::OutStream(o) => write!(f, "{}", o),
         }
     }
 }