Re: [PATCH 1/3] refactor dir_add_name

From: Jeff King <peff@peff.net>
Date: Jun 12, 2007, 12:19 UTC
Message-ID: <20070612121939.GB25145@coredump.intra.peff.net>
In-Reply-To: <7v7iq9lk76.fsf@assigned-by-dhcp.pobox.com>

On Tue, Jun 12, 2007 at 12:13:01AM -0700, Junio C Hamano wrote:

> That looks ugly and also I am curious what the generated
> assembly would look like.  Hopefully the compiler is clever
> enough to generate the same code, but I dunno.

I was curious, too...the assembly generated by gcc -O2 is identical for both versions.

Show 5 quoted lines

> Unless somebody else more versed with C preprocessor tricks
> comes along and offers a better advice, I would go with the
> earlier simpler one with a big fat warning.  I however would
> prefer all caps name for a magic macro like this, whose sole
> point is a huge side effect.

Agreed on the all-caps (in either case) because of the side effects. The more I think about it, I think the inline'd version is better. Multiple evaluation pre-processor bugs are _nasty_ to find, and while the implementation is ugly, it's better to contain the ugliness to one spot than to introduce a dangerous interface that will be used all over.

I'm a bit rusty on my preprocessor tricks, but ISTR that there really isn't a good way to portably avoid the problems. gcc has typeof, which we could use to make temporary copies (which gcc would presumably optimize out), but I imagine we don't want to be gcc-specific. We could conditionally use that construct, but maybe at that point we're getting as ugly as the inline).

-Peff