Skip to content

Commit

Permalink
Make the rust_no_allocation backend deterministic
Browse files Browse the repository at this point in the history
Having non-deterministic generated sources means that incremental
builds would have extra recompilations.

Also pretty-print the generated code.
  • Loading branch information
Colecf committed Nov 10, 2023
1 parent bb25b31 commit d9c2ac4
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 11 deletions.
7 changes: 4 additions & 3 deletions pdl-compiler/src/backends/intermediate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -69,6 +69,7 @@ pub enum ComputedOffsetId<'a> {
TrailerStart,
}

#[derive(PartialEq, Eq, Debug, PartialOrd, Ord)]
pub enum ComputedValue<'a> {
Constant(usize),
CountStructsUpToSize {
Expand All @@ -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>),
Expand Down
8 changes: 2 additions & 6 deletions pdl-compiler/src/backends/rust_no_allocation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 8 additions & 2 deletions pdl-compiler/src/backends/rust_no_allocation/packet_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,18 @@ pub fn generate_packet(
quote! {}
};

// computed_offsets/values are HashMaps, so we need to sort to get deterministic results
let mut computed_offsets_sorted = curr_schema.computed_offsets.iter().collect::<Vec<_>>();
computed_offsets_sorted.sort();
let mut computed_values_sorted = curr_schema.computed_values.iter().collect::<Vec<_>>();
computed_values_sorted.sort();

let computed_getters = empty()
.chain(
curr_schema.computed_offsets.iter().map(|(decl, defn)| decl.declare_fn(defn.compute())),
computed_offsets_sorted.iter().map(|(decl, defn)| decl.declare_fn(defn.compute())),
)
.chain(
curr_schema.computed_values.iter().map(|(decl, defn)| decl.declare_fn(defn.compute())),
computed_values_sorted.iter().map(|(decl, defn)| decl.declare_fn(defn.compute())),
);

let field_getters = fields.iter().map(|field| {
Expand Down
61 changes: 61 additions & 0 deletions pdl-compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,64 @@ 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 sources = ast::SourceDatabase::new();
let mut sources2 = ast::SourceDatabase::new();
let mut sources3 = ast::SourceDatabase::new();

let file = parser::parse_inline(&mut sources, "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 schema = backends::intermediate::generate(&file).unwrap();
let schema2 = backends::intermediate::generate(&file2).unwrap();
let schema3 = backends::intermediate::generate(&file3).unwrap();

let result_1 = backends::rust_no_allocation::generate(&file, &schema).unwrap();
let result_2 = backends::rust_no_allocation::generate(&file2, &schema2).unwrap();
let result_3 = backends::rust_no_allocation::generate(&file3, &schema3).unwrap();

assert_eq!(result_1, result_2);
assert_eq!(result_2, result_3);
}
}

0 comments on commit d9c2ac4

Please sign in to comment.