Re: jsonb is also breaking the rule against nameless unions

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: jsonb is also breaking the rule against nameless unions
Date: 2014-04-02 19:21:26
Message-ID: 20140402192126.GT3750@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-04-02 14:42:39 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-04-02 13:56:40 -0400, Tom Lane wrote:
> >> We really need to get a buildfarm member going that complains about this.
> >> I had hoped to install a sufficiently old gcc version on prairiedog or
> >> dromedary, but didn't have much luck rebuilding ancient gcc releases on
> >> OS X.
>
> > Some experimentation shows that clang's -Wc11-extensions warns about
> > this... If we could get the build on clang warnings free we could use
> > that together with -Werror...
>
> What's it warning about currently?

So, when I manually put a #undef HAVE__STATIC_ASSERT somewhere relevant,
I can compile pg warning free under clang trunk with:
-std=c89 -Wall -Wextra -pedantic
-Wc11-extensions -Wmissing-declarations
-Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers
-Wno-overlength-strings -Wno-variadic-macros -Wno-long-long
-Wno-gnu-statement-expression
without any warnings.

If I add -Wc99-extensions and remove -pedantic it complains about
FLEXIBLE_ARRAY_MEMBER, commas at the end of enumerator lists, extended
field designators (e.g. offsetof(POLYGON, p[0])).

That it warns about FLEXIBLE_ARRAY_MEMBER suggest our configure test for
that could use some improvement. Commas at the enum of enum list are
easily fixed (patch attached).
The extended offsetof bit is a bit more critical, we use that pretty
widely. Luckily it can be disabled with -Wno-extended-offsetof

There's also the valid warning about:
/home/andres/src/postgresql/src/bin/pg_dump/parallel.c:561:22: warning:
initializer for aggregate is not a compile-time constant
[-Wc99-extensions]
int pipefd[2] =
{pipeMW[PIPE_READ], pipeWM[PIPE_WRITE]};
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

pedantic also complains about:

/home/andres/src/postgresql/src/bin/psql/tab-complete.c:815:35: warning:
assigning to 'rl_completion_func_t *'
(aka 'char **(*)(const char *, int, int)') from 'void *' converts
between void pointer and function pointer [-Wpedantic]
rl_attempted_completion_function = (void *) psql_completion;
^ ~~~~~~~~~~~~~~~~~~~~~~~~
which seems valid and solvable.

But there's also:
preproc.y:15039:7: warning: C requires #line number to be less than
32768, allowed as extension [-Wpedantic]
#line 51524 "preproc.c"
^
which seems pretty pointless.

Some thoughts about -Wno-* flags needed:
-Wno-overlength-strings: I don't care.
-Wno-variadic-macros: suggests the configure test is missing a step.
-Wno-long-long: Looks like sloppy coding in seldomly looked at parts to me.
-Wno-gnu-statement-expression: Haven't looked

This doesn't look too bad. At least without -pedantic the warnings seem
sensible after disabling an option or two...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2014-04-02 20:08:47 Re: Doing better at HINTing an appropriate column within errorMissingColumn()
Previous Message Tom Lane 2014-04-02 19:03:47 Re: jsonb is also breaking the rule against nameless unions