Re: pgindent weirdness

On 04/20/2011 05:48 AM, Bruce Momjian wrote:
> Robert Haas wrote:
>> pgindent seems to have muffed it when it comes to BulkInsertStateData:
>>
>> diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
>> index 2849992..72a69e5 100644
>> --- a/src/backend/access/heap/hio.c
>> +++ b/src/backend/access/heap/hio.c
>> @@ -150,7 +150,7 @@ ReadBufferBI(Relation relation, BlockNumber targetBlock,
>>   Buffer
>>   RelationGetBufferForTuple(Relation relation, Size len,
>>                                                    Buffer otherBuffer,
>> int options,
>> -                                                 struct
>> BulkInsertStateData *bistate)
>> +                                                 struct
>> BulkInsertStateData * bistate)
>>   {
>>          bool            use_fsm = !(options&  HEAP_INSERT_SKIP_FSM);
>>          Buffer          buffer = InvalidBuffer;
>>
>> Not sure what happened here exactly...
> BulkInsertStateData is not listed in the typedef list supplied by
> Andrew; see src/tools/pgindent/typedefs.list.  CC'ing him.  This might
> be because the typdef is listed in two files:
>
> /*
>   * state for bulk inserts --- private to heapam.c and hio.c
>   *
>   * If current_buf isn't InvalidBuffer, then we are holding an extra pin
>   * on that buffer.
>   *
>   * "typedef struct BulkInsertStateData *BulkInsertState" is in heapam.h
>   */
>
>


It's tagged as a structure type by objdump, but not as a typedef:

    <1><40055>: Abbrev Number: 4 (DW_TAG_typedef)
    <40056>   DW_AT_name        : (indirect string, offset: 0x6bf6):
    BulkInsertState
    <4005a>   DW_AT_decl_file   : 30
    <4005b>   DW_AT_decl_line   : 32
    <4005c>   DW_AT_type        : <0x40060>
    <1><40060>: Abbrev Number: 7 (DW_TAG_pointer_type)
    <40061>   DW_AT_byte_size   : 8
    <40062>   DW_AT_type        : <0x40066>
    <1><40066>: Abbrev Number: 13 (DW_TAG_structure_type)
    <40067>   DW_AT_name        : (indirect string, offset: 0x66bf):
    BulkInsertStateData
    <4006b>   DW_AT_byte_size   : 16
    <4006c>   DW_AT_decl_file   : 31
    <4006d>   DW_AT_decl_line   : 30
    <4006e>   DW_AT_sibling     : <0x4008b>

I can pull out those too if you want them in the list, but it would 
possibly add a LOT of names to the list.

I did carefully warn you about the need to check the effects of the 
changes when I committed the new list.

It looks like quite a few of the deletions come into this category, for 
example just looking at the diff here 
<https://github.com/postgres/postgres/commit/fe1438da8aa8a45f2cee816eb54841f97d3b2f22#src/tools/pgindent/typedefs.list> 
I see AggHashEntryData, AggStatePerAggData, AllocBlockData, and 
AllocChunkData from among the first few that were deleted and all are in 
the same category.

I wondered if this is some sort of optimizer effect, but building with 
-O0 doesn't seem to affect it.

Note that the list we're using is a composite of dumps from four 
platforms: Linux, FreeBSD, MinGW and Cygwin. What they have in common is 
that they are all using gcc, and a fairly modern version of gcc at that, 
and fairly modern versions of objdump too.

Attached is a partial list of new candidate symbols if we want to pick 
up these extras.

cheers

andrew