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

Commit 1eb3520f authored by David Duarte's avatar David Duarte
Browse files

PDL: Remove Located and Named trait 

The traits were not used for dynamic dispatch nor
for generic code.
The `Named` trait was also directly implemented on
stdlib types (via FieldPath).

As the trait was not giving any advantages over
a direct impl, remove them and put methods
directly on the ast nodes.

Test: compiles
Change-Id: I5ccf498cf985ea4206c2fb42bed5e0d7eb3c5b92
parent f9dff389

tools/pdl/src/ast.rs

+29 −43
Original line number Diff line number Diff line
@@ -168,16 +168,6 @@ pub struct Grammar { 
    pub declarations: Vec<Decl>,

}



/// Implemented for all AST elements.

pub trait Located<'d> {

    fn loc(&'d self) -> &'d SourceRange;

}



/// Implemented for named AST elements.

pub trait Named<'d> {

    fn id(&'d self) -> Option<&'d String>;

}



impl SourceLocation {

    /// Construct a new source location.

    ///
@@ -244,8 +234,34 @@ impl Grammar { 
    }

}



impl<'d> Located<'d> for Field {

    fn loc(&'d self) -> &'d SourceRange {

impl Decl {

    pub fn loc(&self) -> &SourceRange {

        match self {

            Decl::Checksum { loc, .. }

            | Decl::CustomField { loc, .. }

            | Decl::Enum { loc, .. }

            | Decl::Packet { loc, .. }

            | Decl::Struct { loc, .. }

            | Decl::Group { loc, .. }

            | Decl::Test { loc, .. } => loc,

        }

    }



    pub fn id(&self) -> Option<&String> {

        match self {

            Decl::Test { .. } => None,

            Decl::Checksum { id, .. }

            | Decl::CustomField { id, .. }

            | Decl::Enum { id, .. }

            | Decl::Packet { id, .. }

            | Decl::Struct { id, .. }

            | Decl::Group { id, .. } => Some(id),

        }

    }

}



impl Field {

    pub fn loc(&self) -> &SourceRange {

        match self {

            Field::Checksum { loc, .. }

            | Field::Padding { loc, .. }
@@ -261,24 +277,8 @@ impl<'d> Located<'d> for Field { 
            | Field::Group { loc, .. } => loc,

        }

    }

}



impl<'d> Located<'d> for Decl {

    fn loc(&'d self) -> &'d SourceRange {

        match self {

            Decl::Checksum { loc, .. }

            | Decl::CustomField { loc, .. }

            | Decl::Enum { loc, .. }

            | Decl::Packet { loc, .. }

            | Decl::Struct { loc, .. }

            | Decl::Group { loc, .. }

            | Decl::Test { loc, .. } => loc,

        }

    }

}



impl<'d> Named<'d> for Field {

    fn id(&'d self) -> Option<&'d String> {

    pub fn id(&self) -> Option<&String> {

        match self {

            Field::Checksum { .. }

            | Field::Padding { .. }
@@ -296,20 +296,6 @@ impl<'d> Named<'d> for Field { 
    }

}



impl<'d> Named<'d> for Decl {

    fn id(&'d self) -> Option<&'d String> {

        match self {

            Decl::Test { .. } => None,

            Decl::Checksum { id, .. }

            | Decl::CustomField { id, .. }

            | Decl::Enum { id, .. }

            | Decl::Packet { id, .. }

            | Decl::Struct { id, .. }

            | Decl::Group { id, .. } => Some(id),

        }

    }

}



#[cfg(test)]

mod tests {

    use super::*;

tools/pdl/src/lint.rs

+27 −26
Original line number Diff line number Diff line
@@ -22,7 +22,8 @@ pub trait Lintable { 
/// Each field but the last in the chain is a typedef field of a group.

/// The last field can also be a typedef field of a group if the chain is

/// not fully expanded.

type FieldPath<'d> = Vec<&'d Field>;

#[derive(Clone)]

struct FieldPath<'d>(Vec<&'d Field>);



/// Gather information about the full grammar declaration.

struct Scope<'d> {
@@ -90,9 +91,9 @@ impl<'d> std::hash::Hash for &'d Decl { 
    }

}



impl<'d> Located<'d> for FieldPath<'d> {

    fn loc(&'d self) -> &'d SourceRange {

        self.last().unwrap().loc()

impl FieldPath<'_> {

    fn loc(&self) -> &SourceRange {

        self.0.last().unwrap().loc()

    }

}
@@ -147,7 +148,7 @@ impl<'d> PacketScope<'d> { 
    fn insert(&mut self, field: &'d Field, result: &mut LintDiagnostics) {

        match field {

            Field::Checksum { loc, field_id, .. } => {

                self.checksums.insert(field_id.clone(), vec![field]).map(|prev| {

                self.checksums.insert(field_id.clone(), FieldPath(vec![field])).map(|prev| {

                    result.push(

                        Diagnostic::error()

                            .with_message(format!(
@@ -167,7 +168,7 @@ impl<'d> PacketScope<'d> { 
            Field::Padding { .. } | Field::Reserved { .. } | Field::Fixed { .. } => None,



            Field::Size { loc, field_id, .. } | Field::Count { loc, field_id, .. } => {

                self.sizes.insert(field_id.clone(), vec![field]).map(|prev| {

                self.sizes.insert(field_id.clone(), FieldPath(vec![field])).map(|prev| {

                    result.push(

                        Diagnostic::error()

                            .with_message(format!(
@@ -195,7 +196,7 @@ impl<'d> PacketScope<'d> { 
                            ]),

                    )

                }

                self.payload = Some(vec![field]);

                self.payload = Some(FieldPath(vec![field]));

                None

            }
@@ -203,7 +204,7 @@ impl<'d> PacketScope<'d> { 
            | Field::Scalar { loc, id, .. }

            | Field::Typedef { loc, id, .. } => self

                .named

                .insert(id.clone(), vec![field])

                .insert(id.clone(), FieldPath(vec![field]))

                .map(|prev| result.err_redeclared(id, "field", loc, prev.loc())),



            Field::Group { loc, group_id, .. } => {
@@ -314,8 +315,8 @@ impl<'d> PacketScope<'d> { 
        }

        for (id, field) in packet_scope.named.iter() {

            let mut path = vec![group];

            path.extend(field.clone());

            if let Some(prev) = self.named.insert(id.clone(), path) {

            path.extend(field.0.clone());

            if let Some(prev) = self.named.insert(id.clone(), FieldPath(path)) {

                err_redeclared_by_group(

                    result,

                    format!("inserted group redeclares field `{}`", id),
@@ -328,8 +329,8 @@ impl<'d> PacketScope<'d> { 
        // Append group fields to the finalizeed fields.

        for field in packet_scope.fields.iter() {

            let mut path = vec![group];

            path.extend(field.clone());

            self.fields.push(path);

            path.extend(field.0.clone());

            self.fields.push(FieldPath(path));

        }



        // Append group constraints to the caller packet_scope.
@@ -357,7 +358,7 @@ impl<'d> PacketScope<'d> { 
    /// Cleanup scope after processing all fields.

    fn finalize(&mut self, result: &mut LintDiagnostics) {

        // Check field shadowing.

        for f in self.fields.iter().map(|f| f.last().unwrap()) {

        for f in self.fields.iter().map(|f| f.0.last().unwrap()) {

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

                if let Some(prev) = self.all_fields.insert(id.clone(), f) {

                    result.push(
@@ -533,7 +534,7 @@ impl<'d> Scope<'d> { 
                        }

                    }

                    Field::Typedef { type_id, .. } => {

                        lscope.fields.push(vec![f]);

                        lscope.fields.push(FieldPath(vec![f]));

                        match scope.typedef.get(type_id) {

                            None => result.push(

                                Diagnostic::error()
@@ -549,7 +550,7 @@ impl<'d> Scope<'d> { 
                            Some(_) => (),

                        }

                    }

                    _ => lscope.fields.push(vec![f]),

                    _ => lscope.fields.push(FieldPath(vec![f])),

                }

            }
@@ -652,7 +653,7 @@ fn lint_checksum( 
    let checksum_loc = path.loc();

    let field_decl = packet_scope.named.get(field_id);



    match field_decl.and_then(|f| f.last()) {

    match field_decl.and_then(|f| f.0.last()) {

        Some(Field::Typedef { loc: field_loc, type_id, .. }) => {

            // Check declaration type of checksum field.

            match scope.typedef.get(type_id) {
@@ -675,7 +676,7 @@ fn lint_checksum( 
                None => (),

            };

            // Check declaration order of checksum field.

            match field_decl.and_then(|f| f.first()) {

            match field_decl.and_then(|f| f.0.first()) {

                Some(decl) if decl.loc().start > checksum_loc.start => result.push(

                    Diagnostic::error()

                        .with_message("invalid checksum start declaration")
@@ -722,14 +723,14 @@ fn lint_size( 
    let size_loc = path.loc();



    if field_id == "_payload_" {

        return match packet_scope.payload.as_ref().and_then(|f| f.last()) {

        return match packet_scope.payload.as_ref().and_then(|f| f.0.last()) {

            Some(Field::Body { .. }) => result.push(

                Diagnostic::error()

                    .with_message("size field uses undeclared payload field, did you mean _body_ ?")

                    .with_labels(vec![size_loc.primary()]),

            ),

            Some(Field::Payload { .. }) => {

                match packet_scope.payload.as_ref().and_then(|f| f.first()) {

                match packet_scope.payload.as_ref().and_then(|f| f.0.first()) {

                    Some(field) if field.loc().start < size_loc.start => result.push(

                        Diagnostic::error().with_message("invalid size field").with_labels(vec![

                            size_loc
@@ -750,14 +751,14 @@ fn lint_size( 
        };

    }

    if field_id == "_body_" {

        return match packet_scope.payload.as_ref().and_then(|f| f.last()) {

        return match packet_scope.payload.as_ref().and_then(|f| f.0.last()) {

            Some(Field::Payload { .. }) => result.push(

                Diagnostic::error()

                    .with_message("size field uses undeclared body field, did you mean _payload_ ?")

                    .with_labels(vec![size_loc.primary()]),

            ),

            Some(Field::Body { .. }) => {

                match packet_scope.payload.as_ref().and_then(|f| f.first()) {

                match packet_scope.payload.as_ref().and_then(|f| f.0.first()) {

                    Some(field) if field.loc().start < size_loc.start => result.push(

                        Diagnostic::error().with_message("invalid size field").with_labels(vec![

                            size_loc
@@ -780,7 +781,7 @@ fn lint_size( 


    let field = packet_scope.named.get(field_id);



    match field.and_then(|f| f.last()) {

    match field.and_then(|f| f.0.last()) {

        Some(Field::Array { size: Some(_), loc: array_loc, .. }) => result.push(

            Diagnostic::warning()

                .with_message(format!("size field uses array `{}` with static size", field_id))
@@ -807,7 +808,7 @@ fn lint_size( 


        None => result.err_undeclared(field_id, size_loc),

    };

    match field.and_then(|f| f.first()) {

    match field.and_then(|f| f.0.first()) {

        Some(field) if field.loc().start < size_loc.start => {

            result.push(Diagnostic::error().with_message("invalid size field").with_labels(vec![

                    size_loc
@@ -839,7 +840,7 @@ fn lint_count( 
    let count_loc = path.loc();

    let field = packet_scope.named.get(field_id);



    match field.and_then(|f| f.last()) {

    match field.and_then(|f| f.0.last()) {

        Some(Field::Array { size: Some(_), loc: array_loc, .. }) => result.push(

            Diagnostic::warning()

                .with_message(format!("count field uses array `{}` with static size", field_id))
@@ -867,7 +868,7 @@ fn lint_count( 


        None => result.err_undeclared(field_id, count_loc),

    };

    match field.and_then(|f| f.first()) {

    match field.and_then(|f| f.0.first()) {

        Some(field) if field.loc().start < count_loc.start => {

            result.push(Diagnostic::error().with_message("invalid count field").with_labels(vec![

                    count_loc.primary().with_message(format!(
@@ -1043,7 +1044,7 @@ fn lint_field( 
    field: &FieldPath,

    result: &mut LintDiagnostics,

) {

    match field.last().unwrap() {

    match field.0.last().unwrap() {

        Field::Checksum { field_id, .. } => {

            lint_checksum(scope, packet_scope, field, field_id, result)

        }