Make the rust_no_allocation backend deterministic

Having non-deterministic generated sources means that incremental
builds would have extra recompilations.

Also pretty-print the generated code.

pdl-compiler/src/backends/intermediate.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::collections::{hash_map::Entry, HashMap};
+use std::collections::{btree_map::Entry, BTreeMap, HashMap};
 
 use crate::ast;
 use crate::parser;
@@ -23,8 +23,8 @@ pub struct Schema<'a> {
 }
 
 pub struct PacketOrStruct<'a> {
-    pub computed_offsets: HashMap<ComputedOffsetId<'a>, ComputedOffset<'a>>,
-    pub computed_values: HashMap<ComputedValueId<'a>, ComputedValue<'a>>,
+    pub computed_offsets: BTreeMap<ComputedOffsetId<'a>, ComputedOffset<'a>>,
+    pub computed_values: BTreeMap<ComputedValueId<'a>, ComputedValue<'a>>,
     /// whether the parse of this packet needs to know its length,
     /// or if the packet can determine its own length
     pub length: PacketOrStructLength,
@@ -41,7 +41,7 @@ pub struct Enum<'a> {
     pub width: usize,
 }
 
-#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
 pub enum ComputedValueId<'a> {
     // needed for array fields + varlength structs - note that this is in OCTETS, not BITS
     // this always works since array entries are either structs (which are byte-aligned) or integer-octet-width scalars
@@ -54,7 +54,7 @@ pub enum ComputedValueId<'a> {
     Custom(u16),
 }
 
-#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
 pub enum ComputedOffsetId<'a> {
     // these quantities are known by the runtime
     HeaderStart,
@@ -69,6 +69,7 @@ pub enum ComputedOffsetId<'a> {
     TrailerStart,
 }
 
+#[derive(PartialEq, Eq, Debug, PartialOrd, Ord)]
 pub enum ComputedValue<'a> {
     Constant(usize),
     CountStructsUpToSize {
@@ -90,7 +91,7 @@ pub enum ComputedValue<'a> {
     },
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum ComputedOffset<'a> {
     ConstantPlusOffsetInBits(ComputedOffsetId<'a>, i64),
     SumWithOctets(ComputedOffsetId<'a>, ComputedValueId<'a>),
@@ -127,8 +128,8 @@ fn process_enum<'a>(schema: &mut Schema<'a>, id: &'a str, tags: &'a [ast::Tag],
     schema.packets_and_structs.insert(
         id,
         PacketOrStruct {
-            computed_offsets: HashMap::new(),
-            computed_values: HashMap::new(),
+            computed_offsets: BTreeMap::new(),
+            computed_values: BTreeMap::new(),
             length: PacketOrStructLength::Static(width),
         },
     );
@@ -148,8 +149,8 @@ fn compute_getters<'a>(
 ) -> PacketOrStruct<'a> {
     let mut prev_pos_id = None;
     let mut curr_pos_id = ComputedOffsetId::HeaderStart;
-    let mut computed_values = HashMap::new();
-    let mut computed_offsets = HashMap::new();
+    let mut computed_values = BTreeMap::new();
+    let mut computed_offsets = BTreeMap::new();
 
     let mut cnt = 0;
 
@@ -518,7 +519,7 @@ fn compute_getters<'a>(
 }
 
 fn compute_length_to_goal(
-    computed_offsets: &HashMap<ComputedOffsetId, ComputedOffset>,
+    computed_offsets: &BTreeMap<ComputedOffsetId, ComputedOffset>,
     start: ComputedOffsetId,
     goal: ComputedOffsetId,
 ) -> Option<i64> {

pdl-compiler/src/backends/rust_no_allocation/mod.rs

@@ -73,12 +73,8 @@ pub fn generate(file: &parser::ast::File, schema: &Schema) -> Result<String, Str
         .map(|decl| generate_decl(decl, schema, &children))
         .collect::<Result<TokenStream, _>>()?;
 
-    out.push_str(
-        &quote! {
-            #declarations
-        }
-        .to_string(),
-    );
+    let syntax_tree = syn::parse2(declarations).expect("Could not parse code");
+    out.push_str(&prettyplease::unparse(&syntax_tree));
 
     Ok(out)
 }

pdl-compiler/src/lib.rs

@@ -20,3 +20,65 @@ pub mod backends;
 pub mod parser;
 #[cfg(test)]
 pub mod test_utils;
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn rust_no_allocation_output_is_deterministic() {
+        // The generated code should be deterministic, to avoid unnecessary rebuilds during
+        // incremental builds.
+        let src = r#"
+little_endian_packets
+
+enum Enum1 : 8 {
+  ENUM_VARIANT_ONE = 0x01,
+  ENUM_VARIANT_TWO = 0x02,
+}
+
+packet Packet1 {
+  opcode : Enum1,
+  _payload_,
+}
+
+struct Struct1 {
+  handle : 16,
+}
+
+struct Struct2 {
+  _payload_
+}
+
+struct Struct3 {
+  handle : Struct1,
+  value : Struct2,
+}
+
+packet Packet2 : Packet1(opcode = ENUM_VARIANT_ONE) {
+  handle : Struct1,
+  value : Struct2,
+}
+"#
+        .to_owned();
+
+        let mut sources1 = ast::SourceDatabase::new();
+        let mut sources2 = ast::SourceDatabase::new();
+        let mut sources3 = ast::SourceDatabase::new();
+
+        let file1 = parser::parse_inline(&mut sources1, "foo", src.clone()).unwrap();
+        let file2 = parser::parse_inline(&mut sources2, "foo", src.clone()).unwrap();
+        let file3 = parser::parse_inline(&mut sources3, "foo", src).unwrap();
+
+        let schema1 = backends::intermediate::generate(&file1).unwrap();
+        let schema2 = backends::intermediate::generate(&file2).unwrap();
+        let schema3 = backends::intermediate::generate(&file3).unwrap();
+
+        let result1 = backends::rust_no_allocation::generate(&file1, &schema1).unwrap();
+        let result2 = backends::rust_no_allocation::generate(&file2, &schema2).unwrap();
+        let result3 = backends::rust_no_allocation::generate(&file3, &schema3).unwrap();
+
+        assert_eq!(result1, result2);
+        assert_eq!(result2, result3);
+    }
+}