Re: several messages

[Joel E. Denny]

On Sat, 12 Dec 2009, Jonathan Nieder wrote:

> I noticed a few warnings when building mawk:
> 
> compiling parse.c
> y.tab.c:383:6: warning: "YYENABLE_NLS" is not defined
> y.tab.c:1271:6: warning: "YYLTYPE_IS_TRIVIAL" is not defined
> 
> These warnings are not revealing any actual problem (it is perfectly
> reasonable to use #if FOO even if FOO is not defined).  They look
> simple to fix, and a quick Google search shows all kinds of projects
> unnecessarily defining these symbols to work around the warnings, so I
> figure, why not suppress them here at the source?

Sounds good to me.

> These two patches do that.  Perhaps they could be useful.  The result
> still passes the test suite here.

Thanks.

> Jonathan Nieder (2):
>   Check if YYENABLE_NLS is defined before checking its value
>   Do not depend on YYLTYPE_IS_TRIVIAL unless locations are requested
> 
>  data/glr.c    |    4 ++--
>  data/lalr1.cc |    2 +-
>  data/yacc.c   |   10 ++++++----
>  3 files changed, 9 insertions(+), 7 deletions(-)

Good, you're using git.  If you have time, please do the following:

1. Address the comments I've inlined in your patches below.

2. Write a ChangeLog entry.  One for all of the changes you posted seems 
fine.  Please try to follow the format you see in existing ChangeLog 
entries.

3. After committing in your local repo, email us the output of:

  git format-patch --stdout -1

That will make your patch easier to apply.

> diff --git a/src/parse-gram.c b/src/parse-gram.c
> index 4b6d559..0f509e6 100644
> --- a/src/parse-gram.c
> +++ b/src/parse-gram.c

> diff --git a/src/parse-gram.h b/src/parse-gram.h
> index 894b5dd..fcf3255 100644
> --- a/src/parse-gram.h
> +++ b/src/parse-gram.h

When posting patches, it's best if you remove all changes that were 
automatically generated during the build.  We can regenerate those 
ourselves, and we don't want to read them while reviewing your patches.

> Also check if YYLTYPE_IS_TRIVIAL is defined when using its value,
> to avoid warnings if it is not defined.
> ---
> I could not tell from the documentation whether YY_LOCATION_PRINT
> needs to be defined when locations are disabled.  This patches
> leaves it in to be safe.

> diff --git a/data/yacc.c b/data/yacc.c
> index af38314..0d171a7 100644
> --- a/data/yacc.c
> +++ b/data/yacc.c
> @@ -668,7 +668,7 @@ while (YYID (0))
>     we won't break user code: when these are the locations we know.  */
>  
>  #ifndef YY_LOCATION_PRINT
> -# if YYLTYPE_IS_TRIVIAL
> +]b4_locations_if([[# if defined YYLTYPE_IS_TRIVIAL && YYLTYPE_IS_TRIVIAL
>  #  define YY_LOCATION_PRINT(File, Loc)                       \
>       fprintf (File, "%d.%d-%d.%d",                   \
>             (Loc).first_line, (Loc).first_column,     \
> @@ -676,7 +676,9 @@ while (YYID (0))
>  # else
>  #  define YY_LOCATION_PRINT(File, Loc) ((void) 0)
>  # endif
> -#endif
> +]], [[ /* This macro is maintained here in case user code is relying on it. 
> */
> +# define YY_LOCATION_PRINT(File, Loc) ((void) 0)
> +]])[#endif

How does the b4_locations_if part of this change help?  If it isn't really 
necessary, then let's drop it.  As far as I can tell, the addition of 
`defined YYLTYPE_IS_TRIVIAL &&' is sufficient.

Thanks again.