qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Zoltán Kővágó" <dirty.ice.hu@gmail.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC PATCH] osdep: Make MIN/MAX evaluate arguments only once
Date: Sat, 5 Jan 2019 08:34:26 -0600	[thread overview]
Message-ID: <6521a91d-d44f-6116-fd7b-2be7fbac9853@redhat.com> (raw)
In-Reply-To: <758d4c6d-75a3-df55-9128-dad29259da4e@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On 1/5/19 7:48 AM, Eric Blake wrote:

> I'm fine keeping the name MIN/MAX for the common use, but we'd need a
> second pair, maybe named MIN_CONST/MAX_CONST, for use in contexts that
> require a constant (there, both arguments are evaluated twice because it
> is the naive implementation, but that is harmless because both arguments
> are constant and the macro is then usable in places where the smart
> MIN/MAX are not).  I don't know if trying to use __builtin_constant_p in
> the const version would also be worthwhile.  In fact, if
> __builtin_constant_p is powerful enough, perhaps we could use it to
> force a single macro to expand to the naive version if both arguments
> are constant and the smart version otherwise.  I'll give that a shot.

Alas, even though __builtin_constant_p can let the compiler overlook
SOME cases of non-constant code (as in __builtin_constant_p(0 && foo())
returning 1), it is not powerful enough to ignore the dead branch:

$ cat foo.c
#define MIN(a, b)                                               \
    (__builtin_constant_p(a) && __builtin_constant_p(b) ?       \
     (a) < (b) ? (a) : (b) :                                    \
     ({                                                         \
         __auto_type _a = (a) + 0;                              \
         __auto_type _b = (b) + 0;                              \
         _a < _b ? _a : _b;                                     \
     }))

char x[MIN(1, 2)];

int
main(int argc, char **argv)
{
    return MIN(argc, 0);
}
$ gcc -o foo -Wall foo.c
foo.c:4:6: error: braced-group within expression allowed only inside a
function
      ({                                                         \
      ^
foo.c:10:8: note: in expansion of macro ‘MIN’
 char x[MIN(1, 2)];
        ^~~

If anyone can come up with a workaround for single-macro usage, be my
guest.  Otherwise, I'm going with the two-macro solution, with MIN/MAX
for common use, and MIN_CONST/MAX_CONST for constant-context use.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      reply	other threads:[~2019-01-05 14:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-04 15:39 [Qemu-devel] [RFC PATCH] osdep: Make MIN/MAX evaluate arguments only once Eric Blake
2019-01-04 21:47 ` no-reply
2019-01-05  0:23 ` Zoltán Kővágó
2019-01-05 13:48   ` Eric Blake
2019-01-05 14:34     ` Eric Blake [this message]

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=6521a91d-d44f-6116-fd7b-2be7fbac9853@redhat.com \
    --to=eblake@redhat.com \
    --cc=dirty.ice.hu@gmail.com \
    --cc=qemu-devel@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 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).