Re: [PATCH 0/3] yacc: compute the best type for the state number

[Paul Eggert]

On 10/3/19 10:02 PM, Akim Demaille wrote:

> I'm going back to Clang 3.3 and GCC 4.6.  That's all I managed to set up on 
> Travis.  I would also like to try tcc, but only to run the tests, not build 
> bison itself.  Travis also supports MSVC; when I feel brave enough, I'll see if 
> I can use it for Bison.

For what it's worth, I can't build Bison master on Fedora 30 (the current stable 
version of Fedora). Something to do with gettext version numbers. At some point 
I suppose I should file a bug report. I work around the bug with --disable-nls, 
but that's not compatible with --enable-gcc-warnings (so I suppose I should file 
*two* bug reports :-).

Also, I have been trying to build Bison master on Solaris 10 (the oldest Solaris 
still supported) with Oracle Developer Studio 12.6 (the current stable version). 
I found a few glitches and just now installed the attached. I think there will 
be a few glitches more.

> We can introduce a %define variable to enable the new type for 
> columns/locations, and hook it to %require 3.5 to promote the conversion.

Should we let the user specify the type (any integer type, I suppose), or limit 
the user to just 'unsigned' and 'intmax_t'? Perhaps call the type 'yy_pos_num' 
in yacc.c? (Is there a reason it's yy_state_num in yacc.c but yyStateNum in 
glr.c?) Just thinking out loud.


> >    YY_CONVERT_INT_BEGIN
> >    *yyssp = yystate;
> >    YY_CONVERT_INT_END
>
> Why would a warning here be bogus?  It looks to me like a perfect place to use 
> a good old cast, as we're narrowing the type.

I don't like casts, because they mean the compiler won't catch serious errors 
such as mistakenly assigning a pointer to an integer. Other GNU programs 
typically do not enable -Wconversion or -Wimplicit-int-conversion because they 
generate too many false alarms. I figured I could remove the need for casts by 
disabling the misguided warnings.

However, after seeing your diagnostics I suppose that the pragmas are more 
trouble than they're worth. Bison skeletons should be robust even in the 
presence of misguided compiler options like -Wconversion (because some apps are 
foolish enough to use such options :-) and it's such a pain to turn off these 
options portably that casts are probably the way to go in skeletons, despite 
their flaws.
> GCC 8 with ASAN (https://api.travis-ci.org/v3/job/592926526/log.txt):

That URL uses Clang 8, not GCC 8. Of course Bison should work with Clang too, 
but see below.
> > 113. output.at:293: testing Output file name: `~!@#$%^&*()-=_+{}[]|\:;<>, .' ...
> > ../../tests/output.at:293: touch "\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.tmp" || 
> > exit 77
> > ../../tests/output.at:293: COLUMNS=1000; export COLUMNS;  bison --color=no -fno-caret -o 
> > "\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.c" --defines="\`~!@#\$%^&*()-=_+{}[]|\\:;<>, 
> > .'.h" glr.y
> > ../../tests/output.at:293: ls "\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.c" 
> > "\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.h"
> > stdout:
> > `~!@#$%^&*()-=_+{}[]|\:;<>, .'.c
> > `~!@#$%^&*()-=_+{}[]|\:;<>, .'.h
> > ../../tests/output.at:293: $CC $CFLAGS $CPPFLAGS  -c -o glr.o -c 
> > "\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.c"
> > stderr:
> > `~!@#$%^&*()-=_+{}[]|\:;<>, .'.c:1467:37: error: operand of ? changes 
> > signedness: 'ptrdiff_t' (aka 'long') to 'unsigned long' [-Werror,-Wsign-conversion]
> >        ptrdiff_t half_max_capacity = YYSIZEMAX / (2 * sizeof yynewStates[0]);
> >                                      ^~~~~~~~~ ~
> > `~!@#$%^&*()-=_+{}[]|\:;<>, .'.c:132:59: note: expanded from macro 'YYSIZEMAX'
> > #define YYSIZEMAX (PTRDIFF_MAX < SIZE_MAX ? PTRDIFF_MAX : (ptrdiff_t) SIZE_MAX)

This is a bug in Clang, as both operands of ? are ptrdiff_t. You might try 
filing a bug report with the Clang folks.

My experience with Clang warnings has not been great. Clang issues multiple 
bogus warnings when compiling Bison itself, and there's little point to enabling 
warnings, particularly in the test cases since Clang issues a bunch of false 
alarms, so many that it's not worth looking for wheat among the chaff. I suggest 
using --enable-gcc-warnings only with recent versions of GCC, which is my 
practice in other projects. It's too much work to pacify Clang or older GCC 
versions, and there's little benefit to doing so.

> There's also the problem of using __STDC_VERSION__ which might be undefined. 

Sorry about that, and thanks for fixing it.

0001-Port-lexcalc-scan.l-to-Solaris-10.patch
Description: Text Data

0002-Port-ARGMATCH_DEFINE_GROUP-calls-to-C99.patch
Description: Text Data

0003-Avoid-quiet-conversion-of-pointer-to-bool.patch
Description: Text Data