Re: Proposal: Make use of C99 designated initialisers for nulls/values arrays

From: Mark Dilger <hornschnorter(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Joe Nelson <joe(at)begriffs(dot)com>, Isaac Morland <isaac(dot)morland(at)gmail(dot)com>, "Smith, Peter" <peters(at)fast(dot)au(dot)fujitsu(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal: Make use of C99 designated initialisers for nulls/values arrays
Date: 2019-10-02 18:39:22
Message-ID: 4dc0abf0-72bf-fa28-6a16-e6953a359c53@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 10/2/19 11:02 AM, Tom Lane wrote:
> Mark Dilger <hornschnorter(at)gmail(dot)com> writes:
>> On 10/2/19 8:46 AM, Tom Lane wrote:
>>> Right. I think that in general it's bad practice for an initializer
>>> to not specify all fields/elements of the target.
>
>> There are numerous locations in the code that raise warnings when
>> -Wmissing-field-initializers is handed to gcc. See, for example,
>> src/backend/utils/adt/formatting.c where
>> static const KeyWord NUM_keywords[]
>> is initialized, and the code comment above that disclaims the need to
>> initialize is_digit and date_mode. Are you proposing cleaning up all
>> such incomplete initializations within the project?
>
> Hmm. Maybe it's worth doing as a code beautification effort, but
> I'm not volunteering. At the same time, I wouldn't like to make a
> change like this, if it introduces dozens/hundreds of new cases.
>
>> I understand that your INIT_ALL_ZEROS macro does nothing to change
>> whether -Wmissing-field-initializers would raise a warning.
>
> Not sure --- the name of that option suggests that maybe it only
> complains about omitted *struct fields* not omitted *array elements*.

With gcc (Debian 8.3.0-6) 8.3.0

int foo[6] = {0, 1, 2};

does not draw a warning when compiled with this flag.

> If it does complain, is there any way that we could extend the macro
> to annotate usages of it to suppress the warning?

Neither initializing a struct with {0} nor with INIT_ALL_ZEROS draws a
warning either, with my gcc. There are reports online that older
versions of the compiler did, see

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36750

but I don't have an older version to test with just now.

Note that initializing a multi-element struct with {1} does still draw a
warning, and reading the thread above suggests that gcc made a specific
effort to allow initialization to {0} to work without warning as a
special case.

So your proposal for using INIT_ALL_ZEROS is probably good with
sufficiently new compilers, and I'm generally in favor of the proposal,
but I don't think the decree you propose can work unless somebody cleans
up all these other cases that I indicated in my prior email.

(I'm sitting on a few patches until v12 goes out the door from some
conversations with you several months ago, and perhaps I'll include a
patch for this cleanup, too, when time comes for v13 patch sets to be
submitted. My past experience submitting patches shortly before a
release was that they get ignored.)

mark

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-10-02 18:47:22 Re: WIP: Generic functions for Node types using generated metadata
Previous Message Robert Haas 2019-10-02 18:30:08 Re: WIP: Generic functions for Node types using generated metadata