Re: [nft PATCH] parser_bison: Fix for bison < 3.6

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Wed, Jun 10, 2026 at 05:16:39PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jun 10, 2026 at 01:57:09PM +0200, Phil Sutter wrote:
> > Support for 'custom' parse.error value was added in bison-3.6. Fall back
> > to previous value for earlier versions.
> > 
> > This is harder to get right than it seems: On one hand, preprocessor
> > macros can't be used in parser_bison.y's declaration section and
> > automake forbids conditional changes to AM_YFLAGS on the other.
> > 
> > Suggested-by: Jan Kończak <jan.konczak@cs.put.poznan.pl>
> > Fixes: 67b822f2b2624 ("parser_bison: on syntax errors, output expected tokens")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  Makefile.am        |  6 ++++++
> >  configure.ac       | 12 ++++++++++++
> >  src/parser_bison.y |  4 ++--
> >  3 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index fa71e06eefee5..ddf145a87c810 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -164,6 +164,12 @@ AM_CFLAGS = \
> >  	$(NULL)
> >  
> >  AM_YFLAGS = -d -Wno-yacc
> > +if BISON_CUSTOM_ERROR
> > +YACC += -D parse.error=custom -D parse.lac=full
> > +AM_CFLAGS += -DBISON_CUSTOM_ERROR
> > +else
> > +YACC += -D parse.error=verbose
> > +endif
> >  
> >  if BUILD_PROFILING
> >  AM_CFLAGS += --coverage
> > diff --git a/configure.ac b/configure.ac
> > index 0d3ee2ac89f69..b6cad3117a51b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -45,6 +45,18 @@ then
> >          exit 1
> >  fi
> >  
> > +AC_ARG_ENABLE([extended_parser_errors],
> > +	      AS_HELP_STRING([--disable-extended-parser-errors],
> > +			     [Disable use of parse.error=custom and LAC in Bison]),
> > +	      [], [
> > +			enable_extended_parser_errors=no
> > +			AC_SUBST([BISON], [$ac_cv_prog_YACC])
> > +			AX_PROG_BISON_VERSION([3.6],
> > +					      [enable_extended_parser_errors=yes])
> > +	      ])
> > +AM_CONDITIONAL([BISON_CUSTOM_ERROR],
> > +	       [test "x$enable_extended_parser_errors" != xno])
> 
> Can this be made transparent? ie. if bison >= 3.6, then enable it
> always. Otherwise, disable it.

This is the default behaviour (which is the "else" case of that huge
AC_ARG_ENABLE). Users may choose to override the version-based setting
via --{en,dis}able-extended-parser-errors. Fine with you or am I missing
your point?

> Then, include this information here in configure.ac:
> 
> echo "
> nft configuration:
>   cli support:                  ${with_cli}
>   enable debugging symbols:     ${enable_debug}
>   use mini-gmp:                 ${with_mini_gmp}
>   enable man page:              ${enable_man_doc}
>   libxtables support:           ${with_xtables}
>   json output support:          ${with_json}
>   collect profiling data:       ${enable_profiling}"
> 
> and here with -V:
> 
> # nft -V
> nftables v1.1.6 (Commodore Bullmoose #7)
>   cli:          editline
>   json:         yes
>   minigmp:      no
>   libxtables:   yes

Oh right, I keep forgetting about the summary (which is really useful as
reading config.log needs training).

> Maybe add:
> 
>   bison >= 3.6: yes
> 
> or similar?

I'd go with "extended parser errors", to remain consistent with the
config option and also because it calls out what actually changed.

There is one other oddity still which I should check: The configure
script always checks bison version, but 'make dist' tarballs ship with
parser_bison.c and thus don't need bison at all. So this patch might
disable the extended error messages for tarball users which don't have
bison installed.

Cheers, Phil