[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: Re: [PATCH 0/3] yacc: compute the best type for the state number
Date: Sat, 5 Oct 2019 01:48:29 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

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.

   *yyssp = yystate;

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"
`~!@#$%^&*()-=_+{}[]|\:;<>, .'.c
`~!@#$%^&*()-=_+{}[]|\:;<>, .'.h
../../tests/output.at:293: $CC $CFLAGS $CPPFLAGS  -c -o glr.o -c 
"\`~!@#\$%^&*()-=_+{}[]|\\:;<>, .'.c"
`~!@#$%^&*()-=_+{}[]|\:;<>, .'.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'

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.

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

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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]