From: "René Scharfe" <l.s.r@web.de>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Git List <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/5] add SWAP macro
Date: Tue, 31 Jan 2017 22:02:54 +0100 [thread overview]
Message-ID: <676ed19c-0c4e-341e-ba30-1f4a23440088@web.de> (raw)
In-Reply-To: <alpine.DEB.2.20.1701311305030.3469@virtualbox>
Am 31.01.2017 um 13:13 schrieb Johannes Schindelin:
> Hi René,
>
> On Mon, 30 Jan 2017, René Scharfe wrote:
>
>> Am 30.01.2017 um 21:48 schrieb Johannes Schindelin:
>>
>>> The commit you quoted embarrasses me, and I have no excuse for it. I
>>> would love to see that myswap() ugliness fixed by replacing it with a
>>> construct that is simpler, and generates good code even without any
>>> smart compiler.
>>
>> I don't see a way to do that without adding a type parameter.
>
> Exactly. And you know what? I would be very okay with that type parameter.
>
> Coccinelle [*1*] should be able to cope with that, too, mehtinks.
Yes, a semantic patch can turn the type of the temporary variable into a
macro parameter. Programmers would have to type the type, though,
making the macro only half as good.
> It would be trivially "optimized" out of the box, even when compiling with
> Tiny C or in debug mode.
Such a compiler is already slowed down by memset(3) calls for
initializing objects and lack of other optimizations. I doubt a few
more memcpy(3) calls would make that much of a difference.
NB: git as compiled with TCC fails several tests, alas. Builds wickedly
fast, though.
> And it would even allow things like this:
>
> #define SIMPLE_SWAP(T, a, b) do { T tmp_ = a; a = b; b = tmp_; } while (0)
> ...
> uint32_t large;
> char nybble;
>
> ...
>
> if (!(large & ~0xf)) {
> SIMPLE_SWAP(char, nybble, large);
> ...
> }
>
> i.e. mixing types, when possible.
>
> And while I do not necessarily expect that we need anything like this
> anytime soon, merely the fact that it allows for this flexibility, while
> being very readable at the same time, would make it a pretty good design
> in my book.
Such a skinny macro which only hides repetition is kind of attractive
due to its simplicity; I can't say the same about the mixed type example
above, though.
The fat version isn't that bad either even without inlining, includes a
few safety checks and doesn't require us to tell the compiler something
it already knows very well. I'd rather let the machine do the work.
René
next prev parent reply other threads:[~2017-01-31 21:03 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-28 21:13 [PATCH 0/5] introduce SWAP macro René Scharfe
2017-01-28 21:38 ` [PATCH 1/5] add " René Scharfe
2017-01-30 15:39 ` Johannes Schindelin
2017-01-30 16:48 ` René Scharfe
2017-01-30 20:48 ` Johannes Schindelin
2017-01-30 21:46 ` René Scharfe
2017-01-31 12:13 ` Johannes Schindelin
2017-01-31 21:02 ` René Scharfe [this message]
2017-02-01 0:44 ` Ramsay Jones
2017-02-01 11:39 ` Johannes Schindelin
2017-01-30 16:01 ` Johannes Schindelin
2017-01-30 16:59 ` René Scharfe
2017-01-30 18:41 ` Johannes Sixt
2017-01-30 21:03 ` Johannes Schindelin
2017-01-30 22:09 ` René Scharfe
2017-01-30 22:21 ` Brandon Williams
2017-01-31 21:03 ` René Scharfe
2017-01-31 21:35 ` Jeff King
2017-01-31 22:29 ` Junio C Hamano
2017-01-31 22:36 ` Jeff King
2017-02-01 11:28 ` Johannes Schindelin
2017-02-01 11:47 ` Jeff King
2017-02-01 18:06 ` René Scharfe
2017-02-01 18:33 ` Junio C Hamano
2017-02-07 22:04 ` René Scharfe
2017-02-07 22:30 ` Junio C Hamano
2017-02-08 15:14 ` Johannes Schindelin
2017-01-31 12:03 ` Johannes Schindelin
2017-04-24 11:29 ` Jeff King
2017-04-24 11:49 ` Jeff King
2017-04-24 13:13 ` Duy Nguyen
2017-04-28 17:04 ` René Scharfe
2017-04-28 21:49 ` Jeff King
2017-04-29 18:16 ` René Scharfe
2017-04-30 3:11 ` Jeff King
2017-05-02 5:29 ` René Scharfe
2017-01-28 21:40 ` [PATCH 2/5] apply: use " René Scharfe
2017-01-28 21:40 ` [PATCH 3/5] " René Scharfe
2017-01-30 16:03 ` Johannes Schindelin
2017-01-30 17:18 ` René Scharfe
2017-01-30 22:22 ` Junio C Hamano
2017-01-31 21:02 ` René Scharfe
2017-01-28 21:41 ` [PATCH 4/5] diff: " René Scharfe
2017-01-30 16:04 ` Johannes Schindelin
2017-01-30 17:26 ` René Scharfe
2017-01-30 22:22 ` Junio C Hamano
2017-01-28 21:42 ` [PATCH 5/5] graph: " René Scharfe
2017-01-30 16:16 ` Johannes Schindelin
2017-01-30 17:41 ` René Scharfe
2017-01-30 23:20 ` [PATCH 0/5] introduce " Junio C Hamano
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=676ed19c-0c4e-341e-ba30-1f4a23440088@web.de \
--to=l.s.r@web.de \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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 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.