From d3ab8fd4b8c0d9f3ba239d603f740f414192a853 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 10 Nov 2023 14:10:55 -0800 Subject: [PATCH] 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 | 23 +++---- .../src/backends/rust_no_allocation/mod.rs | 8 +-- pdl-compiler/src/lib.rs | 62 +++++++++++++++++++ 3 files changed, 76 insertions(+), 17 deletions(-) diff --git a/pdl-compiler/src/backends/intermediate.rs b/pdl-compiler/src/backends/intermediate.rs index 9c3c231..4c92ae0 100644 --- a/pdl-compiler/src/backends/intermediate.rs +++ b/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, ComputedOffset<'a>>, - pub computed_values: HashMap, ComputedValue<'a>>, + pub computed_offsets: BTreeMap, ComputedOffset<'a>>, + pub computed_values: BTreeMap, 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, + computed_offsets: &BTreeMap, start: ComputedOffsetId, goal: ComputedOffsetId, ) -> Option { diff --git a/pdl-compiler/src/backends/rust_no_allocation/mod.rs b/pdl-compiler/src/backends/rust_no_allocation/mod.rs index 858526f..c98bc12 100644 --- a/pdl-compiler/src/backends/rust_no_allocation/mod.rs +++ b/pdl-compiler/src/backends/rust_no_allocation/mod.rs @@ -73,12 +73,8 @@ pub fn generate(file: &parser::ast::File, schema: &Schema) -> Result>()?; - out.push_str( - "e! { - #declarations - } - .to_string(), - ); + let syntax_tree = syn::parse2(declarations).expect("Could not parse code"); + out.push_str(&prettyplease::unparse(&syntax_tree)); Ok(out) } diff --git a/pdl-compiler/src/lib.rs b/pdl-compiler/src/lib.rs index f9d4314..fe60df8 100644 --- a/pdl-compiler/src/lib.rs +++ b/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); + } +}