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

Commit 5f8df1ad authored by Henri Chataing's avatar Henri Chataing
Browse files

PDL: Change the scoping of packet and group declarations 

Packet, struct, and group previously lived in different scopes,
which allowed packets and structs to have the same name.
This impacts the rust code generation as packets and structs are
disambiguated by adding appropriate suffixes, which is undesirable.

Packets and groups are now declared in the same namespace
as structs and other typedef declarations. An error is raised
if a packet is declared with the same name as another struct or
group declaration.

Test: pdl_inline_tests, validate hci_packets.pdl file
Bug: 228327522
Change-Id: I8055bed118e04de7297e240ac075ca529b5a8f8a
parent fe1986fc

tools/pdl/src/lint.rs

+73 −45
Original line number Diff line number Diff line
@@ -26,15 +26,7 @@ type FieldPath<'d> = Vec<&'d Field>; 


/// Gather information about the full grammar declaration.

struct Scope<'d> {

    // Collection of Group declarations.

    groups: HashMap<String, &'d Decl>,



    // Collection of Packet declarations.

    packets: HashMap<String, &'d Decl>,



    // Collection of Enum, Struct, Checksum, and CustomField declarations.

    // Packet and Group can not be referenced in a Typedef field and thus

    // do not share the same namespace.

    // Collection of Group, Packet, Enum, Struct, Checksum, and CustomField declarations.

    typedef: HashMap<String, &'d Decl>,



    // Collection of Packet, Struct, and Group scope declarations.
@@ -499,10 +491,11 @@ impl<'d> Scope<'d> { 
                _ => (),

            }



            let (parent_id, parent_namespace, fields) = match decl {

                Decl::Packet { parent_id, fields, .. } => (parent_id, &scope.packets, fields),

                Decl::Struct { parent_id, fields, .. } => (parent_id, &scope.typedef, fields),

                Decl::Group { fields, .. } => (&None, &scope.groups, fields),

            let (parent_id, fields) = match decl {

                Decl::Packet { parent_id, fields, .. } | Decl::Struct { parent_id, fields, .. } => {

                    (parent_id.as_ref(), fields)

                }

                Decl::Group { fields, .. } => (None, fields),

                _ => return None,

            };
@@ -513,7 +506,7 @@ impl<'d> Scope<'d> { 
            for f in fields {

                match f {

                    Field::Group { group_id, constraints, .. } => {

                        match scope.groups.get(group_id) {

                        match scope.typedef.get(group_id) {

                            None => result.push(

                                Diagnostic::error()

                                    .with_message(format!(
@@ -522,7 +515,7 @@ impl<'d> Scope<'d> { 
                                    ))

                                    .with_labels(vec![f.loc().primary()]),

                            ),

                            Some(group_decl) => {

                            Some(group_decl @ Decl::Group { .. }) => {

                                // Recurse to flatten the inserted group.

                                if let Some(rscope) = bfs(group_decl, context, scope, result) {

                                    // Inline the group fields and constraints into
@@ -530,6 +523,15 @@ impl<'d> Scope<'d> { 
                                    lscope.inline(scope, rscope, f, constraints.iter(), result)

                                }

                            }

                            Some(_) => result.push(

                                Diagnostic::error()

                                    .with_message(format!(

                                        "invalid group field identifier `{}`",

                                        group_id

                                    ))

                                    .with_labels(vec![f.loc().primary()])

                                    .with_notes(vec!["hint: expected group identifier".to_owned()]),

                            ),

                        }

                    }

                    Field::Typedef { type_id, .. } => {
@@ -554,21 +556,32 @@ impl<'d> Scope<'d> { 
            }



            // Iterate over parent declaration.

            for id in parent_id {

                match parent_namespace.get(id) {

                    None => result.push(

            let parent = parent_id.and_then(|id| scope.typedef.get(id));

            match (decl, parent) {

                (Decl::Packet { parent_id: Some(_), .. }, None)

                | (Decl::Struct { parent_id: Some(_), .. }, None) => result.push(

                    Diagnostic::error()

                        .with_message(format!(

                            "undeclared parent identifier `{}`",

                            parent_id.unwrap()

                        ))

                        .with_labels(vec![decl.loc().primary()])

                        .with_notes(vec![format!("hint: expected {} parent", decl.kind())]),

                ),

                (Decl::Packet { .. }, Some(Decl::Struct { .. }))

                | (Decl::Struct { .. }, Some(Decl::Packet { .. })) => result.push(

                    Diagnostic::error()

                            .with_message(format!("undeclared parent identifier `{}`", id))

                        .with_message(format!("invalid parent identifier `{}`", parent_id.unwrap()))

                        .with_labels(vec![decl.loc().primary()])

                        .with_notes(vec![format!("hint: expected {} parent", decl.kind())]),

                ),

                    Some(parent_decl) => {

                (_, Some(parent_decl)) => {

                    if let Some(rscope) = bfs(parent_decl, context, scope, result) {

                        // Import the parent fields and constraints into the current scope.

                        lscope.inherit(scope, rscope, decl.constraints(), result)

                    }

                }

                }

                _ => (),

            }



            lscope.finalize(result);
@@ -581,7 +594,7 @@ impl<'d> Scope<'d> { 
        let mut context =

            Context::<'d> { list: vec![], visited: HashMap::new(), scopes: HashMap::new() };



        for decl in self.packets.values().chain(self.typedef.values()).chain(self.groups.values()) {

        for decl in self.typedef.values() {

            bfs(decl, &mut context, self, result);

        }
@@ -1215,28 +1228,15 @@ impl Decl { 


impl Grammar {

    fn scope<'d>(&'d self, result: &mut LintDiagnostics) -> Scope<'d> {

        let mut scope = Scope {

            groups: HashMap::new(),

            packets: HashMap::new(),

            typedef: HashMap::new(),

            scopes: HashMap::new(),

        };

        let mut scope = Scope { typedef: HashMap::new(), scopes: HashMap::new() };



        // Gather top-level declarations.

        // Validate the top-level scopes (Group, Packet, Typedef).

        //

        // TODO: switch to try_insert when stable

        for decl in &self.declarations {

            if let Some((id, namespace)) = match decl {

                Decl::Checksum { id, .. }

                | Decl::CustomField { id, .. }

                | Decl::Struct { id, .. }

                | Decl::Enum { id, .. } => Some((id, &mut scope.typedef)),

                Decl::Group { id, .. } => Some((id, &mut scope.groups)),

                Decl::Packet { id, .. } => Some((id, &mut scope.packets)),

                _ => None,

            } {

                if let Some(prev) = namespace.insert(id.clone(), decl) {

            if let Some(id) = decl.id() {

                if let Some(prev) = scope.typedef.insert(id.clone(), decl) {

                    result.err_redeclared(id, decl.kind(), decl.loc(), prev.loc())

                }

            }
@@ -1263,3 +1263,31 @@ impl Lintable for Grammar { 
        result

    }

}



#[cfg(test)]

mod test {

    use crate::ast::*;

    use crate::lint::Lintable;

    use crate::parser::parse_inline;



    macro_rules! grammar {

        ($db:expr, $text:literal) => {

            parse_inline($db, "stdin".to_owned(), $text.to_owned()).expect("parsing failure")

        };

    }



    #[test]

    fn test_packet_redeclared() {

        let mut db = SourceDatabase::new();

        let grammar = grammar!(

            &mut db,

            r#"

        little_endian_packets

        struct Name { }

        packet Name { }

        "#

        );

        let result = grammar.lint();

        assert!(!result.diagnostics.is_empty());

    }

}

tools/pdl/src/parser.rs

+19 −8
Original line number Diff line number Diff line
@@ -506,17 +506,14 @@ fn parse_grammar(root: Node<'_>, context: &Context) -> Result<ast::Grammar, Stri 
    Ok(grammar)

}



/// Parse a new source file.

/// The source file is fully read and added to the compilation database.

/// Returns the constructed AST, or a descriptive error message in case

/// of syntax error.

pub fn parse_file(

/// Parse a PDL grammar text.

/// The grammar is added to the compilation database under the

/// provided name.

pub fn parse_inline(

    sources: &mut ast::SourceDatabase,

    name: String,

    source: String,

) -> Result<ast::Grammar, Diagnostic<ast::FileId>> {

    let source = std::fs::read_to_string(&name).map_err(|e| {

        Diagnostic::error().with_message(format!("failed to read input file '{}': {}", &name, e))

    })?;

    let root = PDLParser::parse(Rule::grammar, &source)

        .map_err(|e| {

            Diagnostic::error()
@@ -528,3 +525,17 @@ pub fn parse_file( 
    let file = sources.add(name, source.clone());

    parse_grammar(root, &(file, &line_starts)).map_err(|e| Diagnostic::error().with_message(e))

}



/// Parse a new source file.

/// The source file is fully read and added to the compilation database.

/// Returns the constructed AST, or a descriptive error message in case

/// of syntax error.

pub fn parse_file(

    sources: &mut ast::SourceDatabase,

    name: String,

) -> Result<ast::Grammar, Diagnostic<ast::FileId>> {

    let source = std::fs::read_to_string(&name).map_err(|e| {

        Diagnostic::error().with_message(format!("failed to read input file '{}': {}", &name, e))

    })?;

    parse_inline(sources, name, source)

}