From: Zhao Liu <zhao1.liu@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-rust@nongnu.org
Subject: Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
Date: Thu, 6 Feb 2025 17:15:33 +0800 [thread overview]
Message-ID: <Z6R9tSD5T5tkCYsP@intel.com> (raw)
In-Reply-To: <CABgObfYc5ccC16=K7KVmdLP-jmvwVbxqv1L6c4Oe55MSK4ndng@mail.gmail.com>
> > > +pub struct MemoryRegionOps<T>(
> > > + bindings::MemoryRegionOps,
> > > + // Note: quite often you'll see PhantomData<fn(&T)> mentioned when discussing
> > > + // covariance and contravariance; you don't need any of those to understand
> > > + // this usage of PhantomData. Quite simply, MemoryRegionOps<T> *logically*
> > > + // holds callbacks that take an argument of type &T, except the type is erased
> > > + // before the callback is stored in the bindings::MemoryRegionOps field.
> > > + // The argument of PhantomData is a function pointer in order to represent
> > > + // that relationship; while that will also provide desirable and safe variance
> > > + // for T, variance is not the point but just a consequence.
> > > + PhantomData<fn(&T)>,
> > > +);
> >
> > Wow, it can be wrapped like this!
>
> I like your enthusiasm but I'm not sure what you refer to. ;) Maybe
> it's worth documenting this pattern, so please tell me more (after
> your holidays).
Throughout the entire holiday, I couldn't think of a better way to
express this. I find it particularly useful when wrapping multiple
callbacks. In the future, I want to explore more use cases where this
pattern can be applied.
> > > +impl MemoryRegion {
> > > + // inline to ensure that it is not included in tests, which only
> > > + // link to hwcore and qom. FIXME: inlining is actually the opposite
> > > + // of what we want, since this is the type-erased version of the
> > > + // init_io function below. Look into splitting the qemu_api crate.
> >
> > Ah, I didn't understand the issue described in this comment. Why would
> > inlining affect the linking of tests?
>
> If you don't inline it, do_init_io will always be linked into the
> tests because it is a non-generic function. The tests then fail to
> link, because memory_region_init_io is undefined.
I find even if I drop the `inline` attribution, the test.rs can still be
compiled (by `make check`), I think it's because test.rs hasn't involved
memory related tests so that do_init_io isn't linked into test.rs.
> This is ugly because do_init_io exists *exactly* to extract the part
> that is not generic. (See
> https://users.rust-lang.org/t/soft-question-significantly-improve-rust-compile-time-via-minimizing-generics/103632/8
> for an example of this; I think there's even a procedural macro crate
> that does that for you, but I can't find it right now).
Thanks! I see. I agree to keep `inline` anyway.
next prev parent reply other threads:[~2025-02-06 8:56 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-17 19:39 [RFC PATCH 00/10] rust: remaining part of qdev bindings Paolo Bonzini
2025-01-17 19:39 ` [PATCH 01/10] rust: qemu-api: add sub-subclass to the integration tests Paolo Bonzini
2025-01-20 16:40 ` Zhao Liu
2025-01-17 19:39 ` [PATCH 02/10] rust: qom: add reference counting functionality Paolo Bonzini
2025-01-26 15:15 ` Zhao Liu
2025-01-29 10:03 ` Paolo Bonzini
2025-02-05 8:28 ` Zhao Liu
2025-01-27 7:57 ` Zhao Liu
2025-01-29 10:16 ` Paolo Bonzini
2025-02-05 9:13 ` Zhao Liu
2025-02-05 9:10 ` Paolo Bonzini
2025-02-05 9:40 ` Zhao Liu
2025-02-06 3:26 ` Zhao Liu
2025-01-17 19:39 ` [PATCH 03/10] rust: qom: add object creation functionality Paolo Bonzini
2025-02-06 7:49 ` Zhao Liu
2025-02-06 7:39 ` Paolo Bonzini
2025-01-17 19:39 ` [PATCH 04/10] rust: callbacks: allow passing optional callbacks as () Paolo Bonzini
2025-01-27 8:41 ` Zhao Liu
2025-01-17 19:39 ` [PATCH 05/10] rust: qdev: add clock creation Paolo Bonzini
2025-02-06 8:15 ` Zhao Liu
2025-01-17 19:39 ` [PATCH 06/10] rust: qom: allow initializing interface vtables Paolo Bonzini
2025-01-27 10:33 ` Zhao Liu
2025-01-17 19:40 ` [PATCH 07/10] rust: qdev: make ObjectImpl a supertrait of DeviceImpl Paolo Bonzini
2025-01-27 9:10 ` Zhao Liu
2025-02-06 8:37 ` Philippe Mathieu-Daudé
2025-01-17 19:40 ` [PATCH 08/10] rust: qdev: switch from legacy reset to Resettable Paolo Bonzini
2025-01-27 10:31 ` Zhao Liu
2025-01-27 18:01 ` Paolo Bonzini
2025-01-28 9:25 ` Zhao Liu
2025-02-06 8:31 ` Zhao Liu
2025-01-17 19:40 ` [PATCH 09/10] rust: bindings: add Sync markers to types referred to by MemoryRegionOps Paolo Bonzini
2025-01-27 10:58 ` Zhao Liu
2025-01-17 19:40 ` [PATCH 10/10] rust: bindings for MemoryRegionOps Paolo Bonzini
2025-01-27 12:12 ` Zhao Liu
2025-01-27 18:11 ` Paolo Bonzini
2025-02-06 9:15 ` Zhao Liu [this message]
2025-02-06 9:15 ` Paolo Bonzini
2025-02-06 8:39 ` Philippe Mathieu-Daudé
2025-02-06 8:46 ` Paolo Bonzini
2025-02-06 10:02 ` Philippe Mathieu-Daudé
2025-02-06 10:19 ` Paolo Bonzini
2025-02-10 10:38 ` Philippe Mathieu-Daudé
2025-01-24 2:46 ` [RFC PATCH 00/10] rust: remaining part of qdev bindings Zhao Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z6R9tSD5T5tkCYsP@intel.com \
--to=zhao1.liu@intel.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-rust@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.