qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 15/23] rust: introduce alternative implementation of offset_o


From: Paolo Bonzini
Subject: Re: [PATCH 15/23] rust: introduce alternative implementation of offset_of!
Date: Mon, 4 Nov 2024 17:03:24 +0100
User-agent: Mozilla Thunderbird

On 11/3/24 10:54, Junjie Mao wrote:

Paolo Bonzini <pbonzini@redhat.com> writes:

From: Junjie Mao <junjie.mao@hotmail.com>

offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
that was found on the Rust forums, and whose author agreed to license as
MIT for use in QEMU.

The alternative allows only one level of field access, but apart
from this can be used just by replacing core::mem::offset_of! with
qemu_api::offset_of!.

The actual implementation of offset_of! is done in a declarative macro,
but for simplicity and to avoid introducing an extra level of indentation,
the trigger is a procedural macro #[derive(offsets)].

The procedural macro is perhaps a bit overengineered, but it helps
introducing some idioms that will be useful in the future as well.

Signed-off-by: Junjie Mao <junjie.mao@hotmail.com>
Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Generally looks good to me. Thanks for integrating this!

It seems Rust does not have builtin support for unit tests expecting
compilation failures. There is a crate (named trybuild [1]) for that
purpose but it requires introducing a dozen of new dependencies (see
below). Do you think it worth the effort? If so, I can take a closer
look and cook something for initial review (probably post 9.2).

     trybuild v1.0.101
     ├── glob v0.3.1
     ├── serde v1.0.210
     ├── serde_derive v1.0.210 (proc-macro)
     │   ├── proc-macro2 v1.0.84 (*)
     │   ├── quote v1.0.36 (*)
     │   └── syn v2.0.66 (*)
     ├── serde_json v1.0.132
     │   ├── itoa v1.0.11
     │   ├── memchr v2.7.4
     │   ├── ryu v1.0.18
     │   └── serde v1.0.210
     ├── target-triple v0.1.3
     ├── termcolor v1.4.1
     └── toml v0.8.19
         ├── serde v1.0.210
         ├── serde_spanned v0.6.8
         │   └── serde v1.0.210
         ├── toml_datetime v0.6.8
         │   └── serde v1.0.210
         └── toml_edit v0.22.22
             ├── indexmap v2.6.0
             │   ├── equivalent v1.0.1
             │   └── hashbrown v0.15.0
             ├── serde v1.0.210
             ├── serde_spanned v0.6.8 (*)
             ├── toml_datetime v0.6.8 (*)
             └── winnow v0.6.20

[1] https://docs.rs/trybuild/latest/trybuild/

[snip]
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index a4bc5d01ee8..c2ea22101e4 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -3,8 +3,34 @@
  // SPDX-License-Identifier: GPL-2.0-or-later

  use proc_macro::TokenStream;
-use quote::quote;
-use syn::{parse_macro_input, DeriveInput};
+use proc_macro2::Span;
+use quote::{quote, quote_spanned};
+use syn::{
+    parse_macro_input, parse_quote, punctuated::Punctuated, token::Comma, 
Data, DeriveInput, Field,
+    Fields, Ident, Type, Visibility,
+};
+
+struct CompileError(String, Span);
+
+impl From<CompileError> for proc_macro2::TokenStream {
+    fn from(err: CompileError) -> Self {
+        let CompileError(msg, span) = err;
+        quote_spanned! { span => compile_error!(#msg); }

The documentation [2] says "there should be no space before the =>
token" and that is by intention to tell that `span` is "evaluated in the
context of proc macro" while those after the arm "in the generated
code". Should we follow that convention (even though the extra white
space does not impact building)?

Ah, forgot to reply about this. Personally I think it's clear enough with the space around both sides of "=>", but if there's agreement on removing the space I don't oppose it.

Paolo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]