Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fdd097d3 authored by Martin Geisler's avatar Martin Geisler Committed by Cherrypicker Worker
Browse files

pdl: Add support for constraints in grand children 

Before, we assumed that we could fully parse a packet based on the
not-yet-parsed bytes from the parent packet. However, this assumption
is wrong when child packets are constrained by fields in the (grand)
parent packets:

    packet Parent {
      a: 8,
      b: 8,
      c: 8,
    }
    packet Child: Parent(a = 10) {
      x: 8,
    }
    packet GrandChild: Child(b = 20) {
      y: 8,
    }
    packet GrandGrandChild: GrandChild(c = 30) {
      z: 8,
    }

Here, we need to pass down the values of b and c when constructing a
Child packet since b will be used to specialize into a GrandChild
packet. Similarly, we need to pass c into GrandChild so that we can
determine if we’re dealing with a GrandGranChild packet.

Test: atest pdl_tests pdl_rust_generator_tests_{le,be}
(cherry picked from https://android-review.googlesource.com/q/commit:1a7675d61a5985b93ac9d650ac3b1fa59e343cef)
Merged-In: Iffffdca4b3601f46d3d13bae61490d1e0b60d8ae
Change-Id: Iffffdca4b3601f46d3d13bae61490d1e0b60d8ae
parent 1d4885b7

tools/pdl/Android.bp

+2 −0
Original line number Diff line number Diff line
@@ -93,6 +93,8 @@ rust_test_host { 
        "tests/generated/packet_decl_fixed_enum_field_little_endian.rs",

        "tests/generated/packet_decl_fixed_scalar_field_big_endian.rs",

        "tests/generated/packet_decl_fixed_scalar_field_little_endian.rs",

        "tests/generated/packet_decl_grand_children_big_endian.rs",

        "tests/generated/packet_decl_grand_children_little_endian.rs",

        "tests/generated/packet_decl_mask_scalar_value_big_endian.rs",

        "tests/generated/packet_decl_mask_scalar_value_little_endian.rs",

        "tests/generated/packet_decl_mixed_scalars_enums_big_endian.rs",

tools/pdl/src/backends/rust.rs

+132 −2
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ 
use crate::{ast, lint};

use heck::ToUpperCamelCase;

use quote::{format_ident, quote};

use std::collections::BTreeSet;

use std::path::Path;



use crate::parser::ast as parser_ast;
@@ -123,6 +124,52 @@ fn top_level_packet<'a>(scope: &lint::Scope<'a>, packet_name: &'a str) -> &'a pa 
    decl

}



fn get_packet_children<'a>(scope: &'a lint::Scope<'_>, id: &str) -> &'a [&'a parser_ast::Decl] {

    scope.children.get(id).map(Vec::as_slice).unwrap_or_default()

}



/// Find all constrained fields in children of `id`.

fn find_constrained_fields<'a>(

    scope: &'a lint::Scope<'a>,

    id: &'a str,

) -> Vec<&'a parser_ast::Field> {

    let mut fields = Vec::new();

    let mut field_names = BTreeSet::new();

    let mut children = Vec::from(get_packet_children(scope, id));



    while let Some(child) = children.pop() {

        if let ast::DeclDesc::Packet { id, constraints, .. }

        | ast::DeclDesc::Struct { id, constraints, .. } = &child.desc

        {

            let packet_scope = &scope.scopes[&scope.typedef[id]];

            for constraint in constraints {

                if field_names.insert(&constraint.id) {

                    fields.push(packet_scope.all_fields[&constraint.id]);

                }

            }

            children.extend(get_packet_children(scope, id));

        }

    }



    fields

}



/// Find parent fields which are constrained in child packets.

///

/// These fields are the fields which need to be passed in when

/// parsing a `id` packet since their values are needed for one or

/// more child packets.

fn find_constrained_parent_fields<'a>(

    scope: &'a lint::Scope<'a>,

    id: &'a str,

) -> impl Iterator<Item = &'a parser_ast::Field> {

    let packet_scope = &scope.scopes[&scope.typedef[id]];

    find_constrained_fields(scope, id).into_iter().filter(|field| {

        let id = field.id().unwrap();

        packet_scope.all_fields.contains_key(id) && !packet_scope.named.contains_key(id)

    })

}



/// Generate code for `ast::Decl::Packet` and `ast::Decl::Struct`

/// values.

fn generate_packet_decl(
@@ -207,6 +254,11 @@ fn generate_packet_decl( 
        unreachable!("Could not find {f:?} in parent chain");

    });



    let parse_fields = find_constrained_parent_fields(scope, id).collect::<Vec<_>>();

    let parse_field_names =

        parse_fields.iter().map(|f| format_ident!("{}", f.id().unwrap())).collect::<Vec<_>>();

    let parse_field_types = parse_fields.iter().map(|f| types::rust_type(f));



    let unconstrained_fields = all_fields

        .iter()

        .filter(|f| !packet_scope.all_constraints.contains_key(f.id().unwrap()))
@@ -291,7 +343,7 @@ fn generate_packet_decl( 
        }

    });



    let children = scope.children.get(id).map(Vec::as_slice).unwrap_or_default();

    let children = get_packet_children(scope, id);

    let has_payload = packet_scope.payload.is_some();

    let has_children_or_payload = !children.is_empty() || has_payload;

    let child =
@@ -402,7 +454,7 @@ fn generate_packet_decl( 
                #conforms

            }



            fn parse(mut #span: &mut Cell<&[u8]>) -> Result<Self> {

            fn parse(mut #span: &mut Cell<&[u8]> #(, #parse_field_names: #parse_field_types)*) -> Result<Self> {

                #field_parser

                Ok(Self {

                    #(#field_names,)*
@@ -609,6 +661,53 @@ mod tests { 
    use crate::test_utils::{assert_snapshot_eq, rustfmt};

    use paste::paste;



    /// Parse a string fragment as a PDL file.

    ///

    /// # Panics

    ///

    /// Panics on parse errors.

    pub fn parse_str(text: &str) -> parser_ast::File {

        let mut db = ast::SourceDatabase::new();

        parse_inline(&mut db, String::from("stdin"), String::from(text)).expect("parse error")

    }



    #[track_caller]

    fn assert_iter_eq<T: std::cmp::PartialEq + std::fmt::Debug>(

        left: impl IntoIterator<Item = T>,

        right: impl IntoIterator<Item = T>,

    ) {

        assert_eq!(left.into_iter().collect::<Vec<T>>(), right.into_iter().collect::<Vec<T>>());

    }



    #[test]

    fn test_find_constrained_parent_fields() {

        let code = "

              little_endian_packets

              packet Parent {

                a: 8,

                b: 8,

                c: 8,

              }

              packet Child: Parent(a = 10) {

                x: 8,

              }

              packet GrandChild: Child(b = 20) {

                y: 8,

              }

              packet GrandGrandChild: GrandChild(c = 30) {

                z: 8,

              }

            ";

        let file = parse_str(code);

        let scope = lint::Scope::new(&file).unwrap();

        let find_fields =

            |id| find_constrained_parent_fields(&scope, id).map(|field| field.id().unwrap());

        assert_iter_eq(find_fields("Parent"), vec![]);

        assert_iter_eq(find_fields("Child"), vec!["b", "c"]);

        assert_iter_eq(find_fields("GrandChild"), vec!["c"]);

        assert_iter_eq(find_fields("GrandGrandChild"), vec![]);

    }



    /// Create a unit test for the given PDL `code`.

    ///

    /// The unit test will compare the generated Rust code for all
@@ -898,4 +997,35 @@ mod tests { 
          }

        "

    );



    test_pdl!(

        packet_decl_grand_children,

        "

          enum Enum16 : 16 {

            A = 1,

            B = 2,

          }



          packet Parent {

              foo: Enum16,

              bar: Enum16,

              baz: Enum16,

              _size_(_payload_): 8,

              _payload_

          }



          packet Child : Parent (foo = A) {

              quux: Enum16,

              _payload_,

          }



          packet GrandChild : Child (bar = A, quux = A) {

              _body_,

          }



          packet GrandGrandChild : GrandChild (baz = A) {

              _body_,

          }

        "

    );

}

tools/pdl/src/backends/rust/parser.rs

+8 −3
Original line number Diff line number Diff line 
use crate::backends::rust::{mask_bits, types};

use crate::backends::rust::{find_constrained_parent_fields, mask_bits, types};

use crate::parser::ast as parser_ast;

use crate::{ast, lint};

use heck::ToUpperCamelCase;
@@ -578,7 +578,7 @@ impl<'a> FieldParser<'a> { 
                            }

                            ast::Constraint { id, tag_id: Some(tag_id), .. } => {

                                // TODO: add `type_id` to `Constraint`.

                                let type_id = match &packet_scope.named[id].desc {

                                let type_id = match &packet_scope.all_fields[id].desc {

                                    ast::FieldDesc::Typedef { type_id, .. } => {

                                        format_ident!("{type_id}")

                                    }
@@ -609,12 +609,17 @@ impl<'a> FieldParser<'a> { 
        });

        let constrained_field_idents =

            constrained_fields.iter().map(|field| format_ident!("{field}"));

        let child_parse_args = children.iter().map(|child| {

            let fields = find_constrained_parent_fields(self.scope, child.id().unwrap())

                .map(|field| format_ident!("{}", field.id().unwrap()));

            quote!(#(, #fields)*)

        });

        let packet_data_child = format_ident!("{}DataChild", self.packet_name);

        self.code.push(quote! {

            let child = match (#(#constrained_field_idents),*) {

                #(#match_values => {

                    let mut cell = Cell::new(payload);

                    let child_data = #child_ids_data::parse(&mut cell)?;

                    let child_data = #child_ids_data::parse(&mut cell #child_parse_args)?;

                    if !cell.get().is_empty() {

                        return Err(Error::InvalidPacketError);

                    }

tools/pdl/src/lint.rs

+1 −1
Original line number Diff line number Diff line
@@ -91,7 +91,7 @@ pub struct PacketScope<'d> { 
    pub fields: Vec<&'d parser::ast::Field>,



    // Constraint declarations gathered from Group inlining.

    constraints: HashMap<String, &'d Constraint>,

    pub constraints: HashMap<String, &'d Constraint>,



    // Local and inherited field declarations. Only named fields are preserved.

    // Saved here for reference for parent constraint resolving.

tools/pdl/tests/generated/packet_decl_grand_children_big_endian.rs

0 → 100644
+877 −0

File added.

Preview size limit exceeded, changes collapsed.