Thread: pgsql: Fix bogus %name-prefix option syntax in all our Bison files.

Fix bogus %name-prefix option syntax in all our Bison files.

%name-prefix doesn't use an "=" sign according to the Bison docs, but it
silently accepted one anyway, until Bison 3.0.  This was originally a
typo of mine in commit 012abebab1bc72043f3f670bf32e91ae4ee04bd2, and we
seem to have slavishly copied the error into all the other grammar files.

Per report from Vik Fearing; analysis by Peter Eisentraut.

Back-patch to all active branches, since somebody might try to build
a back branch with up-to-date tools.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/45b7abe59e9485657ac9380f35d2d917dd0da25b

Modified Files
--------------
contrib/cube/cubeparse.y                |    2 +-
contrib/seg/segparse.y                  |    2 +-
src/backend/bootstrap/bootparse.y       |    2 +-
src/backend/parser/gram.y               |    2 +-
src/backend/replication/repl_gram.y     |    2 +-
src/interfaces/ecpg/preproc/ecpg.header |    2 +-
src/pl/plpgsql/src/pl_gram.y            |    2 +-
src/test/isolation/specparse.y          |    2 +-
8 files changed, 8 insertions(+), 8 deletions(-)

Hi,

On 2014-05-28 19:42:35 +0000, Tom Lane wrote:
> Fix bogus %name-prefix option syntax in all our Bison files.
>
> %name-prefix doesn't use an "=" sign according to the Bison docs, but it
> silently accepted one anyway, until Bison 3.0.  This was originally a
> typo of mine in commit 012abebab1bc72043f3f670bf32e91ae4ee04bd2, and we
> seem to have slavishly copied the error into all the other grammar files.
>
> Per report from Vik Fearing; analysis by Peter Eisentraut.

Are you sure about this? When I saw those warnings first after debian
unstable got bison 3.0 I've read the release notes and interpreted it
differently: By accident *only* the = syntax worked for a long time. Then
somewhere around 2.8 they added the syntax without =. That means that <
2.8 versions are likely not to work anymore.

Greetings,

Andres Freund

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

On 2014-05-28 22:55:28 +0200, Andres Freund wrote:
>
> Hi,
>
> On 2014-05-28 19:42:35 +0000, Tom Lane wrote:
> > Fix bogus %name-prefix option syntax in all our Bison files.
> >
> > %name-prefix doesn't use an "=" sign according to the Bison docs, but it
> > silently accepted one anyway, until Bison 3.0.  This was originally a
> > typo of mine in commit 012abebab1bc72043f3f670bf32e91ae4ee04bd2, and we
> > seem to have slavishly copied the error into all the other grammar files.
> >
> > Per report from Vik Fearing; analysis by Peter Eisentraut.
>
> Are you sure about this? When I saw those warnings first after debian
> unstable got bison 3.0 I've read the release notes and interpreted it
> differently: By accident *only* the = syntax worked for a long time. Then
> somewhere around 2.8 they added the syntax without =. That means that <
> 2.8 versions are likely not to work anymore.

So, I looked this up again. The warnings have been added in:
commit 2062d72deb37782eb2c842e8dd1e606db5accbd1
Author: Theophile Ranquet <ranquet@lrde.epita.fr>
Date:   Thu Oct 18 18:00:51 2012 +0000

while the support for the form without = was added in
commit 02975b9aade52df362645b82a81ac2134290e221
Author: Joel E. Denny <jdenny@ces.clemson.edu>
Date:   Sat Dec 2 01:52:16 2006 +0000

According to git tag --contains the syntax without = has been added in
2.4 (not 2.8 as I'd remembered) which was released 2008-11-02. It's
warning since 3.0 which was released 2013-07-25.

It's imo not realistic to rely on bison >= 2.4, at least not in the
backbranches. Pretty damn annoying. We'll have to live with those
warnings for a couple of years.

Greetings,

Andres Freund

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

Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-05-28 22:55:28 +0200, Andres Freund wrote:
>> On 2014-05-28 19:42:35 +0000, Tom Lane wrote:
>>> Fix bogus %name-prefix option syntax in all our Bison files.

>> Are you sure about this? When I saw those warnings first after debian
>> unstable got bison 3.0 I've read the release notes and interpreted it
>> differently: By accident *only* the = syntax worked for a long time. Then
>> somewhere around 2.8 they added the syntax without =. That means that <
>> 2.8 versions are likely not to work anymore.

> According to git tag --contains the syntax without = has been added in
> 2.4 (not 2.8 as I'd remembered) which was released 2008-11-02. It's
> warning since 3.0 which was released 2013-07-25.

Yeah, that's what the buildfarm is showing: members with bison 2.3 or
less are failing :-(.

> It's imo not realistic to rely on bison >= 2.4, at least not in the
> backbranches. Pretty damn annoying. We'll have to live with those
> warnings for a couple of years.

Agreed; even relatively modern platforms such as OS X 10.9 are still
shipping 2.3, or maybe even lower.  Considering that up to now our
benchmark requirement was bison 1.875, requiring 2.4 is a pretty big
jump just to get rid of a warning.

I guess we have to revert this, and IMO we should also lobby the Bison
people to not emit the deprecation warnings yet.

            regards, tom lane

On 2014-05-28 18:52:22 -0400, Tom Lane wrote:
> I guess we have to revert this

Looks like it.

> and IMO we should also lobby the Bison people to not emit the
> deprecation warnings yet.

That's a good idea. What i've been thinking about is to add
-Wno-deprecated to the bison rule in the interim. Maybe after a
configure test for the option. All deprecation warnings so far seem to
be pretty unhelpful.

Btw, the bison release process and documentation suck. Majorly. The most
efficient way to learn about changes seems to be to look at the git
repository.

Andres

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

Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-05-28 18:52:22 -0400, Tom Lane wrote:
>> and IMO we should also lobby the Bison people to not emit the
>> deprecation warnings yet.

> That's a good idea. What i've been thinking about is to add
> -Wno-deprecated to the bison rule in the interim. Maybe after a
> configure test for the option. All deprecation warnings so far seem to
> be pretty unhelpful.

Meh.  If we just hide them permanently, we're likely to be blindsided
somewhere down the road when they turn a deprecation into an error.

What I was wondering about was if we could modify the .y files when
building with a pre-2.4 bison.  It'd be easy enough to fix this with
sed, say.

            regards, tom lane

On 2014-05-28 19:12:44 -0400, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2014-05-28 18:52:22 -0400, Tom Lane wrote:
> >> and IMO we should also lobby the Bison people to not emit the
> >> deprecation warnings yet.
>
> > That's a good idea. What i've been thinking about is to add
> > -Wno-deprecated to the bison rule in the interim. Maybe after a
> > configure test for the option. All deprecation warnings so far seem to
> > be pretty unhelpful.
>
> Meh.  If we just hide them permanently, we're likely to be blindsided
> somewhere down the road when they turn a deprecation into an error.

I think some bleeding edge buildfarm animal will warn us soon
enough. It's not as if we're able to do much about the deprecations as
is without breaking with older releases.

> What I was wondering about was if we could modify the .y files when
> building with a pre-2.4 bison.  It'd be easy enough to fix this with
> sed, say.

.oO(m4). Should be doable and might actually be interesting for a couple
of other things.

I think I'll just stick a BISONFLAGS=+ -Wno-deprecated in my
Makefile.custom for now. I.e. I am not volunteering.

Greetings,

Andres Freund

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

On 5/28/14, 7:02 PM, Andres Freund wrote:
> That's a good idea. What i've been thinking about is to add
> -Wno-deprecated to the bison rule in the interim. Maybe after a
> configure test for the option. All deprecation warnings so far seem to
> be pretty unhelpful.

Here is a patch.

On 2014-05-29 08:33:05 -0400, Peter Eisentraut wrote:
> On 5/28/14, 7:02 PM, Andres Freund wrote:
> > That's a good idea. What i've been thinking about is to add
> > -Wno-deprecated to the bison rule in the interim. Maybe after a
> > configure test for the option. All deprecation warnings so far seem to
> > be pretty unhelpful.
>
> Here is a patch.

FWIW, I vote for applying something like it. It seems better to collect
the -Wno-deprecated in one place (i.e. configure) instead of having it
in every developer's Makefile.custom. The latter will be harder to get
rid of.
I'd add a comment about why it's been added though. I won't remember why
at least...

Greetings,

Andres Freund

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

Andres Freund <andres@2ndquadrant.com> writes:
> FWIW, I vote for applying something like it. It seems better to collect
> the -Wno-deprecated in one place (i.e. configure) instead of having it
> in every developer's Makefile.custom. The latter will be harder to get
> rid of.

Yeah, that's a good point.

> I'd add a comment about why it's been added though. I won't remember why
> at least...

+1

            regards, tom lane

On Thu, May 29, 2014 at 08:33:05AM -0400, Peter Eisentraut wrote:
> On 5/28/14, 7:02 PM, Andres Freund wrote:
> > That's a good idea. What i've been thinking about is to add
> > -Wno-deprecated to the bison rule in the interim. Maybe after a
> > configure test for the option. All deprecation warnings so far seem to
> > be pretty unhelpful.
>
> Here is a patch.
>

Does this need a comment indicating why it's needed and when it can be
removed?

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> He who writes carelessly confesses thereby at the very outset that he does
> not attach much importance to his own thoughts.
   -- Arthur Schopenhauer