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

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

[Pedro Alves]

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 */
-- 
2.14.4

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

[Simon Marchi]

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

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

[Pedro Alves]

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 );
					  ^

yy_fatal_error looks like this:

 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.

However, the default implementation is still 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 | 14 ++++++++++----
 gdb/yy-remap.h  |  2 ++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3876cd9..c473472 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1875,10 +1875,15 @@ 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.
+#
+# Strictly speaking c-exp.c should therefore depend on Makefile.in,
+# but that was a pretty big annoyance.
 
 .SUFFIXES: .y .l
 .y.c:
@@ -1909,6 +1914,7 @@ po/$(PACKAGE).pot: force
 	        -e 's/\([^x]\)realloc/\1xrealloc/g' \
 	        -e 's/\([ \t;,(]\)free\([ \t]*[&(),]\)/\1xfree\2/g' \
 	        -e 's/\([ \t;,(]\)free$$/\1xfree/g' \
+	        -e 's/\([ \t;,(]\)fprintf/\1parser_fprintf/g' \
 		-e 's/yy_flex_xrealloc/yyxrealloc/g' \
 	      < $@ > $@.new && \
 	    rm -f $@ && \
diff --git a/gdb/yy-remap.h b/gdb/yy-remap.h
index 71fa946..d96f8e6 100644
--- a/gdb/yy-remap.h
+++ b/gdb/yy-remap.h
@@ -90,3 +90,5 @@
 #endif
 
 #define YYFPRINTF parser_fprintf
+
+#define YY_FATAL_ERROR(msg) internal_error (__FILE__, __LINE__, msg)
-- 
2.5.5