Re: [PATCH] For flex: define YY_FATAL_ERROR, rename fprintf -> parser_fprintf

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-03-12 14:21, Pedro Alves wrote:
> Switching GDB to make use of gnulib's C++ namespace support mode
> revealed these direct uses of fprintf in the Ada lexer:
> 
>  In file included from ..../src/gdb/ada-exp.y:731:0:
>  ada-lex.c: In function ‘void yy_fatal_error(const char*)’:
>  ada-lex.c:2358:41: error: call to ‘fprintf’ declared with attribute
> warning: The symbol ::fprintf refers to the system function. Use
> gnulib::fprintf instead. [-Werror]
>      (void) fprintf( stderr, "%s\n", msg );
> 					  ^
> 
> The generated yy_fatal_error looks like this with flex 2.5.35:
> 
>  static void yy_fatal_error (yyconst char* msg )
>  {
> 			 (void) fprintf( stderr, "%s\n", msg );
> 	 exit( YY_EXIT_FAILURE );
>  }
> 
> We can define YY_FATAL_ERROR to override that default implementation.
> 
> I think it's a good idea to do that and call internal_error instead of
> exiting gdb, so this is what this patch does.
> 
> However, the default implementation is still generated and
> unconditionally compiled in, so we need to handle that fprintf somehow
> too.  gnulib's C++ namespace support adds that useful warning shown
> above, breaking the build if we don't do anything here.
> 
> So this commit uses sed to replace fprintf calls with parser_fprintf
> calls, like we already do to rewire malloc to xmalloc, etc.
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* Makefile.in (.l.c): Replace fprintf calls with parser_fprintf
> 	calls.
> 	* yy-remap.h (YY_FATAL_ERROR): Define.
> ---
>  gdb/Makefile.in | 28 ++++++++++++++++++++++++----
>  gdb/yy-remap.h  |  2 ++
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 5614cc3386..aa01c8d38e 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -2457,10 +2457,28 @@ po/$(PACKAGE).pot: force
>  # LANG-exp.c is generated in objdir from LANG-exp.y if it doesn't
>  # exist in srcdir, then compiled in objdir to LANG-exp.o.  If we
>  # said LANG-exp.c rather than ./c-exp.c some makes would
> -# sometimes re-write it into $(srcdir)/c-exp.c.  Remove bogus
> -# decls for malloc/realloc/free which conflict with everything else.
> -# Strictly speaking c-exp.c should therefore depend on
> -# Makefile.in, but that was a pretty big annoyance.
> +# sometimes re-write it into $(srcdir)/c-exp.c.
> +#
> +# Remove bogus decls for malloc/realloc/free which conflict with
> +# everything else.
> +#
> +# Replace calls to fprintf in order to redirect stderr -> gdb_stderr.
> +#
> +# We define YY_FATAL_ERROR to hook in our replacement, however, flex
> +# still defines the default version anyway, as a static function,
> +# leading to -Wunused-function warnings, like:
> +#
> +#  ada-lex.c:2329:13: error: ‘void yy_fatal_error(const char*)’
> defined but not used [-Werror=unused-function]
> +#
> +# Observed with flex 2.5.35.  And then with flex 2.6.1 we have:
> +#
> +#      ada-lex.c:#define yynoreturn __attribute__((__noreturn__))
> +#      ada-lex.c:static void yynoreturn yy_fatal_error (yyconst char* 
> msg )
> +#
> +# Fix that by injecting ATTRIBUTE_UNUSED into yy_fatal_error's 
> prototype.
> +#
> +# Strictly speaking c-exp.c should therefore depend on Makefile.in,
> +# but that was a pretty big annoyance.
> 
>  %.c: %.y
>  	$(ECHO_YACC) $(SHELL) $(YLWRAP) $< y.tab.c $@.tmp -- \
> @@ -2489,6 +2507,8 @@ po/$(PACKAGE).pot: force
>  	        -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
>  	        -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
>  		-e 's/yy_flex_xrealloc/yyxrealloc/g' \
> +		-e 's/\([ \t;,(]\)fprintf/\1parser_fprintf/g' \
> +		-e 's/\(static void\) \(yynoreturn \)\?\(yy_fatal_error\)/\1
> ATTRIBUTE_UNUSED \2\3/g' \
>  	      > $@.new && \
>  	    mv $@.new $@
> 
> diff --git a/gdb/yy-remap.h b/gdb/yy-remap.h
> index cdd0aae8c6..ba62adbbfd 100644
> --- a/gdb/yy-remap.h
> +++ b/gdb/yy-remap.h
> @@ -96,4 +96,6 @@
>  # define YYFPRINTF parser_fprintf
>  #endif
> 
> +#define YY_FATAL_ERROR(msg) internal_error (__FILE__, __LINE__, msg)
> +
>  #endif /* YY_REMAP_H */

This LGTM.

Simon