[PULL 06/29] rust: qemu-macros: switch #[property] parsing to use combinators

Paolo Bonzini posted 29 patches 4 days, 7 hours ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Alistair Francis <alistair@alistair23.me>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Laurent Vivier <laurent@vivier.eu>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>
[PULL 06/29] rust: qemu-macros: switch #[property] parsing to use combinators
Posted by Paolo Bonzini 4 days, 7 hours ago
Since we are going to add more attribute parsing for high-level migration
state macros, use the attrs crate instead of a handwritten parser for
device properties as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/Cargo.lock               | 11 +++++
 rust/qemu-macros/src/lib.rs   | 86 ++++++++++++++---------------------
 rust/qemu-macros/src/tests.rs |  8 ++--
 3 files changed, 49 insertions(+), 56 deletions(-)

diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index eea928621a7..8315f98c46f 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -14,6 +14,16 @@ version = "1.2.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c84fc003e338a6f69fbd4f7fe9f92b535ff13e9af8997f3b14b6ddff8b1df46d"
 
+[[package]]
+name = "attrs"
+version = "0.2.9"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2a207d40f43de65285f3de0509bb6cb16bc46098864fce957122bbacce327e5f"
+dependencies = [
+ "proc-macro2",
+ "syn",
+]
+
 [[package]]
 name = "bilge"
 version = "0.2.0"
@@ -188,6 +198,7 @@ dependencies = [
 name = "qemu_macros"
 version = "0.1.0"
 dependencies = [
+ "attrs",
  "proc-macro2",
  "quote",
  "syn",
diff --git a/rust/qemu-macros/src/lib.rs b/rust/qemu-macros/src/lib.rs
index 830b4326985..7ab18061776 100644
--- a/rust/qemu-macros/src/lib.rs
+++ b/rust/qemu-macros/src/lib.rs
@@ -3,10 +3,14 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 use proc_macro::TokenStream;
-use quote::{quote, quote_spanned, ToTokens};
+use quote::{quote, quote_spanned};
 use syn::{
-    parse::Parse, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
-    token::Comma, Data, DeriveInput, Error, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token,
+    parse::{Parse, ParseStream},
+    parse_macro_input, parse_quote,
+    punctuated::Punctuated,
+    spanned::Spanned,
+    token::Comma,
+    Attribute, Data, DeriveInput, Error, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token,
     Variant,
 };
 mod bits;
@@ -159,61 +163,37 @@ enum DevicePropertyName {
     Str(syn::LitStr),
 }
 
-#[derive(Debug)]
+impl Parse for DevicePropertyName {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+        let lo = input.lookahead1();
+        if lo.peek(syn::LitStr) {
+            Ok(Self::Str(input.parse()?))
+        } else if lo.peek(syn::LitCStr) {
+            Ok(Self::CStr(input.parse()?))
+        } else {
+            Err(lo.error())
+        }
+    }
+}
+
+#[derive(Default, Debug)]
 struct DeviceProperty {
     rename: Option<DevicePropertyName>,
     defval: Option<syn::Expr>,
 }
 
-impl Parse for DeviceProperty {
-    fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
-        let _: syn::Token![#] = input.parse()?;
-        let bracketed;
-        _ = syn::bracketed!(bracketed in input);
-        let attribute = bracketed.parse::<syn::Ident>()?;
-        debug_assert_eq!(&attribute.to_string(), "property");
-        let mut retval = Self {
-            rename: None,
-            defval: None,
-        };
-        let content;
-        _ = syn::parenthesized!(content in bracketed);
-        while !content.is_empty() {
-            let value: syn::Ident = content.parse()?;
-            if value == "rename" {
-                let _: syn::Token![=] = content.parse()?;
-                if retval.rename.is_some() {
-                    return Err(syn::Error::new(
-                        value.span(),
-                        "`rename` can only be used at most once",
-                    ));
-                }
-                if content.peek(syn::LitStr) {
-                    retval.rename = Some(DevicePropertyName::Str(content.parse::<syn::LitStr>()?));
-                } else {
-                    retval.rename =
-                        Some(DevicePropertyName::CStr(content.parse::<syn::LitCStr>()?));
-                }
-            } else if value == "default" {
-                let _: syn::Token![=] = content.parse()?;
-                if retval.defval.is_some() {
-                    return Err(syn::Error::new(
-                        value.span(),
-                        "`default` can only be used at most once",
-                    ));
-                }
-                retval.defval = Some(content.parse()?);
-            } else {
-                return Err(syn::Error::new(
-                    value.span(),
-                    format!("unrecognized field `{value}`"),
-                ));
-            }
+impl DeviceProperty {
+    fn parse_from(&mut self, a: &Attribute) -> syn::Result<()> {
+        use attrs::{set, with, Attrs};
+        let mut parser = Attrs::new();
+        parser.once("rename", with::eq(set::parse(&mut self.rename)));
+        parser.once("default", with::eq(set::parse(&mut self.defval)));
+        a.parse_args_with(&mut parser)
+    }
 
-            if !content.is_empty() {
-                let _: syn::Token![,] = content.parse()?;
-            }
-        }
+    fn parse(a: &Attribute) -> syn::Result<Self> {
+        let mut retval = Self::default();
+        retval.parse_from(a)?;
         Ok(retval)
     }
 }
@@ -235,7 +215,7 @@ fn derive_device_or_error(input: DeriveInput) -> Result<proc_macro2::TokenStream
             f.attrs
                 .iter()
                 .filter(|a| a.path().is_ident("property"))
-                .map(|a| Ok((f.clone(), syn::parse2(a.to_token_stream())?)))
+                .map(|a| Ok((f.clone(), DeviceProperty::parse(a)?)))
         })
         .collect::<Result<Vec<_>, Error>>()?;
     let name = &input.ident;
diff --git a/rust/qemu-macros/src/tests.rs b/rust/qemu-macros/src/tests.rs
index 9ab7eab7f37..00a106612fc 100644
--- a/rust/qemu-macros/src/tests.rs
+++ b/rust/qemu-macros/src/tests.rs
@@ -60,7 +60,7 @@ struct DummyState {
                 migrate_clock: bool,
             }
         },
-        "unrecognized field `defalt`"
+        "Expected one of `default` or `rename`"
     );
     // Check that repeated attributes are not allowed:
     derive_compile_fail!(
@@ -73,7 +73,8 @@ struct DummyState {
                 migrate_clock: bool,
             }
         },
-        "`rename` can only be used at most once"
+        "Duplicate argument",
+        "Already used here",
     );
     derive_compile_fail!(
         derive_device_or_error,
@@ -85,7 +86,8 @@ struct DummyState {
                 migrate_clock: bool,
             }
         },
-        "`default` can only be used at most once"
+        "Duplicate argument",
+        "Already used here",
     );
     // Check that the field name is preserved when `rename` isn't used:
     derive_compile!(
-- 
2.51.0