Re: Unportable coding in reorderbuffer.h

On 2014-03-05 19:12:15 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-03-05 17:40:56 -0500, Tom Lane wrote:
> >> By the time you get done fixing the portability issue, I suspect you
> >> won't have a union at all for the first case.
>
> > You might be right. I'd rather not leak the internal enum values to the
> > public though, so there are two choices:
> > * define as enum, but store values in the that aren't actually valid
> > members. IIRC that's legal, even if not pretty. Anything outside
> > reorderbuffer.c doesn't ever see the values that aren't enum values.
> > * define it as an int, but suggest casting to the enum inside switch()
> > in examples/docs.
>
> Meh. I think you're being *way* too cute here. I'd just put all the
> values in one enum declaration, and use comments and/or naming convention
> to mark some of them as internal and not meant to be used outside the
> reorderbuffer stuff. That will for one thing allow gdb to print all
> the values properly. And somebody who's bound and determined to violate
> the abstraction could do it anyway, no?

The reason I'd like to avoid the internal members in the public enum
isn't so much that I want to avoid the internal names being visible to
the outside, but that I find it very helpful to see compile time
warnings about public enum values not being handled in output plugins. I
am pretty sure we're going to add further features in the not too far
away futures and it seems nicer to be able to warn that way.

But I guess it's too much work, for not enough benefit :(

> >> What drew my attention to it was an older gcc version complaining
> >> thusly: [errors]
>
> > And there I was, suprised it hadn't turned the buildfarm red.
>
> I'm surprised too; I had thought we still had some critters running
> hoary compilers. We need to do something about that if we actually
> believe in C90-compiler support.

What version was the gcc that triggered the error?

I have to admit that I am really not interested in supporting gcc 2.95 ,
that thing is just too old (nearly 15 years!) and buggy. But it's not a small
step from desupporting 2.95 to having gcc 3.4 (protosciurus, narwahl) as
the minimum. And it should be a conscious not implicit decision.

It'd be really helpful to have some more buildfarm animals running
real-world relevant older compilers. I'd be surprised if this is the
only such issue lurking around.

As I've compiled it anyway while thinking about gcc versions, here's the
lifetimes of various gcc releases:

version first version last version
---
2.95 July 31, 1999 March 16, 2001
3.0 June 18, 2001 December 20, 2001
3.1 May 15, 2002 July 27, 2002
3.2 August 14, 2002 April 25, 2003
3.3 May 14, 2003 May 3, 2005
3.4 April 18, 2004 March 6, 2006
4.0 April 20, 2005 January 31, 2007
4.1 February 28, 2006 February 13, 2007
4.2 May 13, 2007 May 19, 2008
4.3 March 5, 2008 Jun 27, 2011
4.4 April 21, 2009 March 13, 2012
4.5 April 14, 2010 Jul 2, 2012
4.6 March 25, 2011 April 12, 2013
4.7 March 22, 2012 -
4.8 March 22, 2013 -
---

I personally think it's time to dump some older compiler versions, and
adopt at least individual C99 features (e.g. static inlines).

Greetings,

Andres Freund