qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: qemu-devel@nongnu.org, flwu@google.com, berrange@redhat.com,
	 peter.maydell@linaro.org, rkir@google.com
Subject: Re: [PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
Date: Thu, 27 Jun 2024 10:48:55 +0200	[thread overview]
Message-ID: <CABgObfYuf3Bbu_VZt+6Me6HpPhMo04hm_dyFvQwTCKdm3uEuAw@mail.gmail.com> (raw)
In-Reply-To: <fqdc2.8zr6z4s9qcnm@linaro.org>

On Thu, Jun 27, 2024 at 10:38 AM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Thu, 27 Jun 2024 00:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >On Tue, Jun 25, 2024 at 9:17 PM Manos Pitsidianakis
> ><manos.pitsidianakis@linaro.org> wrote:
> >> >Move the macro to compiler.h and, while at it, move it under #ifndef
> >> >__cplusplus to emphasize that it uses C-only constructs.  A C++ version
> >> >of typeof_strip_qual() using type traits is possible[1], but beyond the
> >> >scope of this patch because the little C++ code that is in QEMU does not
> >> >use QAPI.
> >>
> >> Should we add an #else to provide a fallback for cplusplus until the
> >> alternative is merged?
> >
> >As the commit message says, I don't think we need to include the C++
> >code in QEMU since it would be essentially dead.
>
> The #ifndef __cplusplus part stood out for me, if it's not required for
> the qemu build then it's similarly unnecessary. Thinking out loud here.

Yeah, the patch had actually two purposes, only one of which is
explicit in the commit message because the other is more relevant to
Google than to the upstream project:

1) Google wants some help making QEMU headers compilable with C++,
which is generally not a goal of the project but not something we want
to hinder. And if that brings to our attention things that could be
improved in C as well, why not.

2) it is true that typeof_strip_qual() is useful in other places than
qemu/atomic.h

Since qemu/compiler.h already has #ifdef __cplusplus in various
places, and could reasonably be included in the few C++ files that
QEMU has, it makes sense to have a #ifndef __cplusplus in
qemu/compiler.h (unlike in qemu/atomic.h).  Personally I prefer to
have a little more cognitive load in common headers such as
qemu/osdep.h and qemu/compiler.h, than to have different styles (e.g.
no qemu/osdep.h inclusion) in C++ sources, even though most likely
I'll never touch those C++ sources.

The #ifndef is not absolutely necessary---if we removed it, Google
could work around that with "#ifdef __cplusplus / #undef
typeof_strip_qual". It is mostly for documentation purposes to point
out the C-only compiler builtins, and because if anyone ended up using
typeof_strip_qual in C++ code the error message would be very very
long.

Paolo



  reply	other threads:[~2024-06-27  8:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-25 11:18 [PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH() Paolo Bonzini
2024-06-25 18:31 ` Richard Henderson
2024-06-25 19:12 ` Manos Pitsidianakis
2024-06-26 21:30   ` Roman Kiryanov
2024-06-26 21:32   ` Paolo Bonzini
2024-06-27  8:34     ` Manos Pitsidianakis
2024-06-27  8:48       ` Paolo Bonzini [this message]
2024-06-27  8:53         ` Manos Pitsidianakis

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=CABgObfYuf3Bbu_VZt+6Me6HpPhMo04hm_dyFvQwTCKdm3uEuAw@mail.gmail.com \
    --to=pbonzini@redhat.com \
    --cc=berrange@redhat.com \
    --cc=flwu@google.com \
    --cc=manos.pitsidianakis@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rkir@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).