Raising our compiler requirements for 9.6

On Wed, Jul 1, 2015 at 9:14 AM, Andres Freund <andres@anarazel.de> wrote:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it and we go through some ugly lengths to avoid
relying on inline functions in headers. It's a feature that has been
there in most compilers long before C99.

My feeling is that we shouldn't go the full way to C99 because there's
still common compilers without a complete coverage. But individual
features are fine.

I am in full agreement.

The list of features, in the order of perceived importance, that might
be worthwhile thinking about are:
* static inline
* variadic macros
* designated initializers (e.g. somestruct foo = { .bar = 3 } )
* // style comments (I don't care, but it comes up often enough ...)

I don't want to add // style comments, FWIW.

What is the state of support like for variadic macros and designated
initializers? Unlike static inline, I am not aware that they are
something that was widely implemented before C99 was formalized.
--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

Also, I think there are some other non-gcc animals that nominally allow
"static inline" but will generate warnings when such functions are
unreferenced in a particular compile (that's what the "quiet inline"
configure test is about). That would be hugely annoying for development,
though maybe we don't care too much if it's only a build target.

I'm not against requiring static inline; it would be a huge improvement
really. But we should not fool ourselves that this comes at zero
compatibility cost.

The list of features, in the order of perceived importance, that might
be worthwhile thinking about are:
* static inline
* variadic macros
* designated initializers (e.g. somestruct foo = { .bar = 3 } )
* // style comments (I don't care, but it comes up often enough ...)

Of these I think only the first is really worth breaking portability
for.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Geoghegan <pg@heroku.com> writes:

On Wed, Jul 1, 2015 at 9:14 AM, Andres Freund <andres@anarazel.de> wrote:

The list of features, in the order of perceived importance, that might
be worthwhile thinking about are:
* static inline
* variadic macros
* designated initializers (e.g. somestruct foo = { .bar = 3 } )
* // style comments (I don't care, but it comes up often enough ...)

I don't want to add // style comments, FWIW.

I concur with that one. I think the portability risk is nil (even
pademelon's compiler takes // without complaint, which surprised me
when I realized it). Rather, I think the argument for continuing to
disallow // has to do with maintaining code style consistency. A mishmash
of // and /* comments looks like heck. (Note, BTW, that pgindent will
forcibly convert // to /* in some though seemingly not all cases.)

As far as "static inline" goes, it occurs to me after more thought that
there really is zero risk of build failures if we start relying on that,
because we already have the "#define inline as empty" trick in configure.
What would happen, on a compiler like pademelon's, is that you'd get a
whole lot of warnings about unreferenced static functions. And maybe some
bloat in the executable, if the compiler is not smart enough to drop those
functions from its output. I think both of these effects are probably
acceptable considering what a small minority of platforms would have any
issue.

A potentially larger issue is that I think we have places where frontend
code is #include'ing backend headers that contain macros we might wish
to convert to inline functions, and that will not work if the macros
contain references to backend functions or global variables. But we could
solve that by refactoring headers where necessary.

What is the state of support like for variadic macros and designated
initializers? Unlike static inline, I am not aware that they are
something that was widely implemented before C99 was formalized.

I agree that the value-for-portability-risk tradeoff doesn't look
great for these features.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 07/01/2015 01:33 PM, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

Other reasoning aside, pademelon is running an HPUX version that is 10
years old. I don't think we should care.

JD

--
Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

01.07.2015, 23:33, Tom Lane kirjoitti:

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

HP-UX 10.20 was released in 1996, last shipped in July 2002 and
unsupported since July 2003. Assuming the new features aren't going to
be used in release branches PG 9.5 is probably going to support that
platform longer than there's hardware to run it..

But we should not fool ourselves that this comes at zero
compatibility cost.

But compatibility with obsolete platforms doesn't come with zero cost
either -- and judging from the fact that no one noticed until now that
you couldn't even configure PG 9.0 .. 9.3 on recent Solaris 10 releases
(which I believe was the most popular proprietary Unix) suggests that
not a whole lot of people care about those platforms anymore.

/ Oskari

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

"Joshua D. Drake" <jd@commandprompt.com> writes:

On 07/01/2015 01:33 PM, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

Other reasoning aside, pademelon is running an HPUX version that is 10
years old. I don't think we should care.

Try 16. The reason I run it is not because anyone cares about actually
running Postgres on such a machine; it's just so that we will know when
we are breaking compatibility with ancient C compilers. I brought it up
merely to refute Andres' claim that we do not have buildfarm coverage
of the case.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-07-01 16:33:24 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

Oh. I'd gone through all the animals somewhere in the middle of the 9.5
cycle. pademelon was either dormant or not yet reincarnated back then.

I'll go through all again then. Interesting that anole's compiler
supports inlines.

Also, I think there are some other non-gcc animals that nominally allow
"static inline" but will generate warnings when such functions are
unreferenced in a particular compile (that's what the "quiet inline"
configure test is about). That would be hugely annoying for development,
though maybe we don't care too much if it's only a build target.

I know that 4c8aa8b5aea1e032f569222d4b6c1019e84622dc "fixed" that test
on a number of older platforms. Apparently even 10+ years ago some
compiler authors added the smarts to recognize whether static inlines
where declared in a header (don't warn) or in a .c file (warn).

I'm not against requiring static inline; it would be a huge improvement
really. But we should not fool ourselves that this comes at zero
compatibility cost.

It's not zero, but I think it's quite small.

The list of features, in the order of perceived importance, that might
be worthwhile thinking about are:
* static inline
* variadic macros
* designated initializers (e.g. somestruct foo = { .bar = 3 } )
* // style comments (I don't care, but it comes up often enough ...)

Of these I think only the first is really worth breaking portability
for.

If variadic macros, or even designated initializers, were supported by
the same set of animals in the buildfarm I'd happily use it, but they're
unfortunately not...

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-07-01 23:39:06 +0200, Andres Freund wrote:

On 2015-07-01 16:33:24 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

At the very least I think we should start to rely on 'static inline's
working. There is not, and hasn't been for a while, any buildfarm animal
that does not support it

pademelon doesn't.

Oh. I'd gone through all the animals somewhere in the middle of the 9.5
cycle. pademelon was either dormant or not yet reincarnated back then.

I'll go through all again then. Interesting that anole's compiler
supports inlines.

Here are a list of animals and what they support. I left out several
redundant entries (don't need 30 reports of newish linux + gcc
supporting everything).

animal OS compiler inline quiet inline varargs

anole HPUX 11.31 HP C/aC++A.06.25 y y y
axolotl fed-pi gcc-4.9 y y y
baiji vista MSVC 2005 y y y
binturong solaris 10 sun studio 12.3 y y y
baiji win 8 MSVC 2012 y y y
brolga cygwin gcc-4.3 y y
castoroides solaris 10 sun studio 12.1 y y y
catamount OSX 10.8 llvm-gcc 4.2 y y y
coypu netbsd 5.1 gcc-4.1 y y y
currawong win xp MSVC 2008 y y y
damselfly illumos 201311 gcc-4.7 y y y
dingo solaris 10 gcc-4.9 y y y
dromedary OSX 10.6 gcc-4.2 y y y
dunlin gento 13.0 icc 13.1 y y y
elk freebsd 7.1 gcc-4.2 y y y
frogmouth win xp gcc-4.5 y y y
gaur HP-UX 10.20 gcc-2.95 y y y
gharial HP-UX 11.31 gcc-4.6 y y y
locust OSX 10.5 gcc-4.0 y y y
mallard fedora 21 clang-3.5 y y y
mouflon openbsd 5.7 gcc-4.2 y n* y
narwhal win 2003 gcc-3.4 y y y
okapi gentoo 12.14 icc 11.1 y y y
olinguito openbsd 5.3 gcc-4.2 y n* y
opossum netbsd 5.2 gcc-4.1 y y y
orangutan OSX 10.4 gcc-4.0 y y y
pademelon HP-UX 10.2 HP C Compiler 10.32 n n n
pika netbsd 4.99 gcc-4.1 y y ?**
protosciurus solaris 10 gcc-3.4 y y y
rover_firef OmniOS gcc-4.6 ?*** ?*** ?***
smew debian 6.0 clang 2.9 y y y
spoonbill openbsd 3.6 gcc-3.3 y y y

* fails due to idiotic warnings we can't do much about:
/usr/local/lib/libxml2.so.15.1: warning: strcpy() is almost always misused, please use strlcpy()
/usr/local/lib/libxml2.so.15.1: warning: strcat() is almost always misused, please use strlcat()
/usr/local/lib/libxslt.so.3.8: warning: sprintf() is often misused, please use snprintf()
** hasn't run since check was added, but gcc-4.1 supports
*** doesn't report any details, too old buildfarm client? All supported by 4.6

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-07-02 00:15:14 +0200, Andres Freund wrote:

animal OS compiler inline quiet inline varargs

brolga cygwin gcc-4.3 y y

4.3 obviously supports varargs. Human error.

pademelon HP-UX 10.2 HP C Compiler 10.32 n n n

Since, buildfarm/quiet inline test issues aside, pademelon is the only
animal not supporting inlines and varargs, I think we should just go
ahead and start to use both.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:

Since, buildfarm/quiet inline test issues aside, pademelon is the only
animal not supporting inlines and varargs, I think we should just go
ahead and start to use both.

I'm good with using inlines, since as I pointed out upthread, that won't
actually break anything. I'm much less convinced that varargs macros
represent a winning tradeoff. Using those *will* irredeemably break
pre-C99 compilers, and AFAICS we do not have an urgent need for them.

(BTW, where are you drawing the conclusion that all these compilers
support varargs? I do not see a configure test for it.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-07-01 19:05:08 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

Since, buildfarm/quiet inline test issues aside, pademelon is the only
animal not supporting inlines and varargs, I think we should just go
ahead and start to use both.

I'm good with using inlines, since as I pointed out upthread, that won't
actually break anything. I'm much less convinced that varargs macros
represent a winning tradeoff. Using those *will* irredeemably break
pre-C99 compilers, and AFAICS we do not have an urgent need for them.

Well, I'll happily take that.

(BTW, where are you drawing the conclusion that all these compilers
support varargs? I do not see a configure test for it.)

There is, although not in all branches: PGAC_C_VA_ARGS. We optionally
use vararg macros today, for elog (b853eb9), so I assume it works ;)

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 1, 2015 at 6:24 PM, Andres Freund <andres@anarazel.de> wrote:

On 2015-07-02 00:15:14 +0200, Andres Freund wrote:

animal OS compiler inline quiet inline varargs

brolga cygwin gcc-4.3 y y

4.3 obviously supports varargs. Human error.

pademelon HP-UX 10.2 HP C Compiler 10.32 n n n

Since, buildfarm/quiet inline test issues aside, pademelon is the only
animal not supporting inlines and varargs, I think we should just go
ahead and start to use both.

So far this thread is all about the costs of desupporting compilers
that don't have these features, and you're making a good argument
(that I think we all agree with) that the cost is small. But you
haven't really said what the benefits are.

In the case of static inline, the main problem with the status quo
AFAICS is that you have to do a little dance any time you'd otherwise
use a static inline function (for those following along at home, "git
grep INCLUDE_DEFINITIONS src/include" to see how this is done). Now,
it is obviously the case that the first time somebody has to do this
dance, they will be somewhat inconvenienced, but once you know how to
do it, it's not incredibly complicated. So, just to play the devil's
advocate here, who really cares? The way we're doing it right now
works everywhere and is as efficient on each platform as the compiler
on that platform can support. I accept that there is some developer
convenience to not having to worry about the INCLUDE_DEFINITIONS
thing, and maybe that's a sufficient reason to do it, but is that the
only benefit we're hoping to get?

I'd consider an argument for adopting one of these features to be much
stronger if were accompanied by arguments like:

- If we require feature X, we can reduce the size of the generated
code and improve performance.
- If we require feature X, we can reduce the risk of bugs.
- If we require feature X, static analyzers will be able to understand
our code better.

If everybody else here says "working around the lack of feature X is
too much of a pain in the butt and we want to adopt it purely too make
development easier", I'm not going to sit here and try to fight the
tide. But I personally don't find such arguments all that exciting.
It's not at all clear to me that static inline or variadic macros
would make our lives meaningfully better, and like Peter, I think //
comments are a not something we should adopt, because one style is
better than two. Designated initializers would have meaningful
documentation value and might help to decrease the risk of bugs, so
that almost seems more interesting to me than static inline. We'd
need to check what pgindent thinks of them, though, and how much
platform coverage we'd be surrendering.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-07-02 11:46:25 -0400, Robert Haas wrote:

In the case of static inline, the main problem with the status quo
AFAICS is that you have to do a little dance any time you'd otherwise
use a static inline function (for those following along at home, "git
grep INCLUDE_DEFINITIONS src/include" to see how this is done).
Now,
it is obviously the case that the first time somebody has to do this
dance, they will be somewhat inconvenienced, but once you know how to
do it, it's not incredibly complicated.

I obviously know the schema (having introduced it in pg), but I think
the costs are actually rather significant. It makes development more
complex, it makes things considerably harder to follow, and it's quite
annoying to test (manually redefine PG_USE_INLINE, recompile whole
tree).

Several times people also argued against using inlines with that trick
because it's slowing down older platforms.

So, just to play the devil's advocate here, who really cares?

I consider our time one of the most scarce resources around.

I'd consider an argument for adopting one of these features to be much
stronger if were accompanied by arguments like:

- If we require feature X, we can reduce the size of the generated
code and improve performance.

Converting some of the bigger macros (I tested fastgetattr) to inliens,
actually does both.

See also http://archives.postgresql.org/message-id/4407.1435763473%40sss.pgh.pa.us

Now, all that is possible with the INCLUDE_DEFINITIONS stuff, but it
makes it considerably more expensive time/complexity wise.

If everybody else here says "working around the lack of feature X is
too much of a pain in the butt and we want to adopt it purely too make
development easier", I'm not going to sit here and try to fight the
tide. But I personally don't find such arguments all that exciting.

I find that an odd attitude.

I think // comments are a not something we should adopt, because one
style is better than two

I don't particularly care about // comments either. They do have the
advantage of allowing three more characters of actual content in one
line comments ...

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

So far this thread is all about the costs of desupporting compilers
that don't have these features, and you're making a good argument
(that I think we all agree with) that the cost is small. But you
haven't really said what the benefits are.

I made the same remark with respect to varargs macros, and I continue
to think that the cost-benefit ratio there is pretty marginal.

However, I do think that there's a case to be made for adopting static
inline. The INCLUDE_DEFINITIONS dance is very inconvenient, so it
discourages people from using static inlines over macros, even in cases
where the macro approach is pretty evil (usually, because of multiple-
evaluation hazards). If we allowed static inlines then we could work
towards having a safer coding standard along the lines of "thou shalt
not write macros that evaluate their arguments more than once".
So the benefits here are easier development and less risk of bugs.
On the other side of the ledger, the costs are pretty minimal; we will
not actually break any platforms, at worst we'll make them unpleasant
to develop on because of lots of warning messages. We have some platforms
like that already, and it's not been a huge problem.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 2, 2015 at 12:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

So far this thread is all about the costs of desupporting compilers
that don't have these features, and you're making a good argument
(that I think we all agree with) that the cost is small. But you
haven't really said what the benefits are.

I made the same remark with respect to varargs macros, and I continue
to think that the cost-benefit ratio there is pretty marginal.

However, I do think that there's a case to be made for adopting static
inline. The INCLUDE_DEFINITIONS dance is very inconvenient, so it
discourages people from using static inlines over macros, even in cases
where the macro approach is pretty evil (usually, because of multiple-
evaluation hazards). If we allowed static inlines then we could work
towards having a safer coding standard along the lines of "thou shalt
not write macros that evaluate their arguments more than once".
So the benefits here are easier development and less risk of bugs.
On the other side of the ledger, the costs are pretty minimal; we will
not actually break any platforms, at worst we'll make them unpleasant
to develop on because of lots of warning messages. We have some platforms
like that already, and it's not been a huge problem.

OK, so do we want to rip out all instances of the static inline dance
in favor of more straightforward coding? Do we then shut pandemelon
and any other affected buildfarm members down as unsupported, or what?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-08-04 15:20:14 -0400, Robert Haas wrote:

OK, so do we want to rip out all instances of the static inline dance
in favor of more straightforward coding? Do we then shut pandemelon
and any other affected buildfarm members down as unsupported, or what?

I think all that happens is that they'll log a couple more warnings
about defined but unused static functions. configure already defines
inline away if not supported.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:

On 2015-08-04 15:20:14 -0400, Robert Haas wrote:

OK, so do we want to rip out all instances of the static inline dance
in favor of more straightforward coding? Do we then shut pandemelon
and any other affected buildfarm members down as unsupported, or what?

I think all that happens is that they'll log a couple more warnings
about defined but unused static functions. configure already defines
inline away if not supported.

Right. We had already concluded that this would be safe to do, it's
just a matter of somebody being motivated to do it.

I'm not sure that there's any great urgency about changing the instances
that exist now; the real point of this discussion is that we will allow
new code to use static inlines in headers.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-08-04 15:45:44 -0400, Tom Lane wrote:

I'm not sure that there's any great urgency about changing the instances
that exist now; the real point of this discussion is that we will allow
new code to use static inlines in headers.

I agree that we don't have to (and probably shouldn't) make the required
configure changes and change definitions. But I do think some of the
current macro usage deserves to be cleaned up at some point. I,
somewhere during 9.4 or 9.5, redefined some of the larger macros into
static inlines and it both reduced the binary size and gave minor
performance benefits.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Aug 4, 2015 at 3:55 PM, Andres Freund <andres@anarazel.de> wrote:

On 2015-08-04 15:45:44 -0400, Tom Lane wrote:

I'm not sure that there's any great urgency about changing the instances
that exist now; the real point of this discussion is that we will allow
new code to use static inlines in headers.

I agree that we don't have to (and probably shouldn't) make the required
configure changes and change definitions. But I do think some of the
current macro usage deserves to be cleaned up at some point. I,
somewhere during 9.4 or 9.5, redefined some of the larger macros into
static inlines and it both reduced the binary size and gave minor
performance benefits.

We typically recommend that people write their new code like the
existing code. If we say that the standards for new code are now
different from old code in this one way, I don't think that's going to
be very helpful to anyone.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers