Re: [PATCH 2/7] move setting of object->type to alloc_* functions

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

On 13/07/14 07:41, Jeff King wrote:
> On Sat, Jul 12, 2014 at 02:05:39PM -0400, Jeff King wrote:
> 
>>> I don't particularly like 'flag' here. (not a massive dislike, mind you:)
>>>
>>> Perhaps: flag->object_type, type->node_type?
>>> Or, if that's too verbose, maybe just: flag->type, type->node?
>>
>> Me either, but as you noticed, type was taken. Your suggestions seem
>> fine. We could also just do away with the macro as discussed earlier (we
>> already do in the commit_node case, anyway...).
> 
> Thinking on this more, writing out the definitions is the only sane
> thing to do here, now that alloc_commit_node does not use the macro.
> Otherwise you are inviting people to modify the macro, but fail to
> notice that the commit allocator also needs updating.

Hmm, well I could argue that using the macro for all allocators, apart
from alloc_commit_node(), clearly shows which allocator is the odd-man
out (and conversely, that all others are the same)! :-P

No, I don't think this is a telling advantage; I don't think it makes
that much difference. (six of one, half-a-dozen of the other).

BTW, I tested the previous series on Linux 32-bit, Cygwin 32-bit, MinGW
32-bit and Cygwin 64-bit. (I can't test on Linux 64-bit, since I can't
get Linux installed on my new laptop :( ). Admittedly, the testing on
MinGW and Cygwin was only fairly light (it takes *hours* to run the full
testsuite, and I just don't have the time).

I was slightly concerned, when reading through this new series, that the
alloc_node() function may no longer be inlined in the new allocators.
However, I have just tested on Linux (only using gcc this time), and it
was just fine. I will test the new series on the above systems later
(probably tomorrow) but don't expect to find any problems.

> 
> Here's a re-roll. The interesting bit is the addition of the second
> patch (but the rest needed to be rebased on top).

Yep, this looks good. Thanks!

> 
>   [1/8]: alloc.c: remove the alloc_raw_commit_node() function
>   [2/8]: alloc: write out allocator definitions
>   [3/8]: move setting of object->type to alloc_* functions
>   [4/8]: parse_object_buffer: do not set object type
>   [5/8]: add object_as_type helper for casting objects
>   [6/8]: alloc: factor out commit index
>   [7/8]: object_as_type: set commit index
>   [8/8]: diff-tree: avoid lookup_unknown_object

ATB,
Ramsay Jones