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

Commit fe1322e6 authored by Anna Bauza's avatar Anna Bauza Committed by Gerrit Code Review
Browse files

Merge changes from topic "revert-2586907-MQJYHMXFGR"

* changes:
  Revert "pdl: Generate snapshots using prettyplease"
  Revert "pdl: Generate canonical tests with prettyplease"
  Revert "pdl: Format generated code using prettyplease"
  Revert "pdl: Remove rustfmt from no_alloc test"
parents f6f26372 b2569c27
Loading
Loading
Loading
Loading
+38 −10
Original line number Diff line number Diff line
@@ -15,7 +15,6 @@ rust_defaults {
        "libcodespan_reporting",
        "libheck",
        "libpest",
        "libprettyplease",
        "libproc_macro2",
        "libquote",
        "libserde",
@@ -122,7 +121,15 @@ rust_test_host {
        "libpaste",
    ],
    test_suites: ["general-tests"],
    enabled: false, // rustfmt is only available on x86.
    arch: {
        x86_64: {
            enabled: true,
        },
    },
    data: [
        ":rustfmt",
        ":rustfmt.toml",
        ":pdl_generated_files",
    ],
}
@@ -146,8 +153,13 @@ rust_test_host {

genrule_defaults {
    name: "pdl_rust_generator_defaults",
    cmd: "$(location :pdl) --output-format rust $(in) > $(out)",
    tools: [":pdl"],
    cmd: "set -o pipefail;" +
        " $(location :pdl) --output-format rust $(in) |" +
        " $(location :rustfmt) > $(out)",
    tools: [
        ":pdl",
        ":rustfmt",
    ],
    defaults_visibility: [
        "//external/uwb/src",
        "//packages/modules/Bluetooth:__subpackages__",
@@ -240,7 +252,6 @@ rust_binary_host {
    name: "pdl_generate_tests",
    srcs: ["src/bin/generate-canonical-tests.rs"],
    rustlibs: [
        "libprettyplease",
        "libproc_macro2",
        "libquote",
        "libserde",
@@ -250,20 +261,32 @@ rust_binary_host {
    ],
}

genrule_defaults {
    name: "pdl_rust_generator_src_defaults",
    tools: [
        ":pdl_generate_tests",
        ":rustfmt",
    ],
}

genrule {
    name: "pdl_rust_generator_tests_le_src",
    cmd: "$(location :pdl_generate_tests) $(in) pdl_le_backend > $(out)",
    cmd: "set -o pipefail;" +
        " $(location :pdl_generate_tests) $(in) pdl_le_backend |" +
        " $(location :rustfmt) > $(out)",
    srcs: ["tests/canonical/le_test_vectors.json"],
    out: ["le_canonical.rs"],
    tools: [":pdl_generate_tests"],
    defaults: ["pdl_rust_generator_src_defaults"],
}

genrule {
    name: "pdl_rust_generator_tests_be_src",
    cmd: "$(location :pdl_generate_tests) $(in) pdl_be_backend > $(out)",
    cmd: "set -o pipefail;" +
        " $(location :pdl_generate_tests) $(in) pdl_be_backend |" +
        " $(location :rustfmt) > $(out)",
    srcs: ["tests/canonical/be_test_vectors.json"],
    out: ["be_canonical.rs"],
    tools: [":pdl_generate_tests"],
    defaults: ["pdl_rust_generator_src_defaults"],
}

rust_test_host {
@@ -381,8 +404,13 @@ python_test_host {
// Defaults for the rust_noalloc backend
genrule_defaults {
    name: "pdl_rust_noalloc_generator_defaults",
    cmd: "$(location :pdl) --output-format rust_no_alloc $(in) > $(out)",
    tools: [":pdl"],
    cmd: "set -o pipefail;" +
        " $(location :pdl) --output-format rust_no_alloc $(in) |" +
        " $(location :rustfmt) > $(out)",
    tools: [
        ":pdl",
        ":rustfmt",
    ],
}

// Generate the rust_noalloc backend srcs against the little-endian test vectors
+0 −1
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ quote = "1.0.21"
serde_json = "1.0.86"
argh = "0.1.7"
syn = "1.0.102"
prettyplease = "0.1.25"

[dependencies.serde]
version = "1.0.145"
+19 −15
Original line number Diff line number Diff line
@@ -964,20 +964,24 @@ fn generate_decl(
    scope: &lint::Scope<'_>,
    file: &analyzer_ast::File,
    decl: &analyzer_ast::Decl,
) -> proc_macro2::TokenStream {
) -> String {
    match &decl.desc {
        ast::DeclDesc::Packet { id, .. } => generate_packet_decl(scope, file.endianness.value, id),
        ast::DeclDesc::Packet { id, .. } => {
            generate_packet_decl(scope, file.endianness.value, id).to_string()
        }
        ast::DeclDesc::Struct { id, parent_id: None, .. } => {
            // TODO(mgeisler): handle structs with parents. We could
            // generate code for them, but the code is not useful
            // since it would require the caller to unpack everything
            // manually. We either need to change the API, or
            // implement the recursive (de)serialization.
            generate_struct_decl(scope, file.endianness.value, id)
            generate_struct_decl(scope, file.endianness.value, id).to_string()
        }
        ast::DeclDesc::Enum { id, tags, width } => {
            generate_enum_decl(id, tags, *width, false).to_string()
        }
        ast::DeclDesc::Enum { id, tags, width } => generate_enum_decl(id, tags, *width, false),
        ast::DeclDesc::CustomField { id, width: Some(width), .. } => {
            generate_custom_field_decl(id, *width)
            generate_custom_field_decl(id, *width).to_string()
        }
        _ => todo!("unsupported Decl::{:?}", decl),
    }
@@ -988,18 +992,18 @@ fn generate_decl(
/// The code is not formatted, pipe it through `rustfmt` to get
/// readable source code.
pub fn generate(sources: &ast::SourceDatabase, file: &analyzer_ast::File) -> String {
    let mut code = String::new();

    let source = sources.get(file.file).expect("could not read source");
    let preamble = preamble::generate(Path::new(source.name()));
    code.push_str(&preamble::generate(Path::new(source.name())));

    let scope = lint::Scope::new(file);
    let decls = file.declarations.iter().map(|decl| generate_decl(&scope, file, decl));
    let code = quote! {
        #preamble
    for decl in &file.declarations {
        code.push_str(&generate_decl(&scope, file, decl));
        code.push_str("\n\n");
    }

        #(#decls)*
    };
    let syntax_tree = syn::parse2(code).expect("Could not parse code");
    prettyplease::unparse(&syntax_tree)
    code
}

#[cfg(test)]
@@ -1008,7 +1012,7 @@ mod tests {
    use crate::analyzer;
    use crate::ast;
    use crate::parser::parse_inline;
    use crate::test_utils::{assert_snapshot_eq, format_rust};
    use crate::test_utils::{assert_snapshot_eq, rustfmt};
    use paste::paste;

    /// Parse a string fragment as a PDL file.
@@ -1091,7 +1095,7 @@ mod tests {
                    let actual_code = generate(&db, &file);
                    assert_snapshot_eq(
                        &format!("tests/generated/{name}_{endianness}.rs"),
                        &format_rust(&actual_code),
                        &rustfmt(&actual_code),
                    );
                }
            }
+21 −25
Original line number Diff line number Diff line
@@ -12,11 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use quote::quote;
use std::path::Path;

use crate::quote_block;

/// Generate the file preamble.
pub fn generate(path: &Path) -> proc_macro2::TokenStream {
pub fn generate(path: &Path) -> String {
    let mut code = String::new();
    let filename = path.file_name().unwrap().to_str().expect("non UTF-8 filename");
    // TODO(mgeisler): Make the  generated code free from warnings.
    //
    // The code either needs
@@ -31,35 +34,22 @@ pub fn generate(path: &Path) -> proc_macro2::TokenStream {
    // to the generated code. We cannot add the module-level attribute
    // here because of how the generated code is used with include! in
    // lmp/src/packets.rs.
    let filename = path.file_name().unwrap().to_str().expect("non UTF-8 filename");
    let module_doc_string = format!(" @generated rust packets from {filename}.");
    // TODO(mgeisler): the doc comment below should be an outer
    // comment (#![doc = ...]). However, people include the generated
    // code in the middle of another module via include_str!:
    //
    // fn before() {}
    // include_str!("generated.rs")
    // fn after() {}
    //
    // It is illegal to have a //! comment in the middle of a file. We
    // should refactor such usages to instead look like this:
    //
    // fn before() {}
    // mod foo { include_str!("generated.rs") }
    // use foo::*;
    // fn after() {}
    quote! {
        #[doc = #module_doc_string]
    code.push_str(&format!("// @generated rust packets from {filename}\n\n"));

    code.push_str(&quote_block! {
        use bytes::{Buf, BufMut, Bytes, BytesMut};
        use std::convert::{TryFrom, TryInto};
        use std::cell::Cell;
        use std::fmt;
        use std::sync::Arc;
        use thiserror::Error;
    });

    code.push_str(&quote_block! {
        type Result<T> = std::result::Result<T, Error>;
    });

    code.push_str(&quote_block! {
        /// Private prevents users from creating arbitrary scalar values
        /// in situations where the value needs to be validated.
        /// Users can freely deref the value, but only the backend
@@ -73,7 +63,9 @@ pub fn generate(path: &Path) -> proc_macro2::TokenStream {
                &self.0
            }
        }
    });

    code.push_str(&quote_block! {
        #[derive(Debug, Error)]
        pub enum Error {
            #[error("Packet parsing failed")]
@@ -93,22 +85,26 @@ pub fn generate(path: &Path) -> proc_macro2::TokenStream {
            #[error("expected child {expected}, got {actual}")]
            InvalidChildError { expected: &'static str, actual: String },
        }
    });

    code.push_str(&quote_block! {
        pub trait Packet {
            fn to_bytes(self) -> Bytes;
            fn to_vec(self) -> Vec<u8>;
        }
    }
    });

    code
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::test_utils::{assert_snapshot_eq, format_rust};
    use crate::test_utils::{assert_snapshot_eq, rustfmt};

    #[test]
    fn test_generate_preamble() {
        let actual_code = generate(Path::new("some/path/foo.pdl")).to_string();
        assert_snapshot_eq("tests/generated/preamble.rs", &format_rust(&actual_code));
        let actual_code = generate(Path::new("some/path/foo.pdl"));
        assert_snapshot_eq("tests/generated/preamble.rs", &rustfmt(&actual_code));
    }
}
+11 −10
Original line number Diff line number Diff line
@@ -128,16 +128,17 @@ fn generate_unit_tests(input: &str, packet_names: &[&str], module_name: &str) {
    }

    // TODO(mgeisler): make the generated code clean from warnings.
    let code = quote! {
        #![allow(warnings, missing_docs)]

    println!("#![allow(warnings, missing_docs)]");
    println!();
    println!(
        "{}",
        &quote! {
            use #module::Packet;
            use serde_json::json;

            #(#tests)*
    };
    let syntax_tree = syn::parse2::<syn::File>(code).expect("Could not parse {code:#?}");
    println!("{}", prettyplease::unparse(&syntax_tree));
        }
    );
}

fn main() {
Loading