Re: [PATCH v7 06/13] pack-objects: move in_pack out of struct object_entry

From: Jeff King <peff@peff.net>
Date: Mar 31, 2018, 10:20 UTC
Message-ID: <20180331102007.GC32290@sigill.intra.peff.net>
In-Reply-To: <CACsJy8Dma-xrDcK1vQ-s4F+Dwt4oWOQ89yr33OPJdO1g0ryQWw@mail.gmail.com>

On Sat, Mar 31, 2018 at 06:51:10AM +0200, Duy Nguyen wrote:

Show 14 quoted lines

> >> +#define IN_PACK(obj) oe_in_pack(&to_pack, obj)
> >
> > How come this one gets a macro, but the earlier conversions don't?
> >
> > I guess the problem is that oe_in_pack() is defined in the generic
> > pack-objects.h, but &to_pack is only in builtin/pack-objects.c?
> >
> > I wonder if it would be that bad to just say oe_in_pack(&to_pack, obj)
> > everywhere. It's longer, but it makes the code slightly less magical to
> > read.
> 
> Longer was exactly why I added these macros (with the hope that the
> macro upper case names already ring a "it's magical" bell). Should I
> drop all these macros? Some code becomes a lot more verbose though.

I'm on the fence. I agree that the macro screams "magical". I just sometimes see a macro and think something really weird and unfunction-like is going on. But really we're just replacing a default parameter.

So I dunno. If you get rid of the macros and I look at it, I give even odds that I'll say "yech, put them back!". :)

-Peff