Hi Paul, Thanks for the quick response!
> Le 5 oct. 2019 à 10:48, Paul Eggert <[email protected]> a écrit : > > 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. I currently don't have Gettext 0.20 available on my distro, and bootstrap pulls gnulib-po which installs files from Gettext 0.20. So I have to `cp po/Makefile.in.in gnulib-po`. That's also what the CI does. > 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 :-). I had not noticed this :( > 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. It's great that you tried that! Nelson, at some point, had sent me logs about this platform, but that was a long time ago. Maybe some of these fixes should make it into `maint`, in case we have to release 3.4.3. >> 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'? I'd rather play it safe and offer only a binary. I am tired to writing tests for convoluted cases :) > 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. Well, the reason is that yacc.c and the rest followed "our" style, but when Paul Hilfinger contributed glr.c, no-one told him to follow the GCS, and the code remained as is. I would not object to migrate it to the "official" style, but I did not felt obligated to do so. Even if that does mean that on occasions, for consistency between glr.c and yacc.c, I write snake_case_functions in glr.c, which ends up being self-inconsistent. >>> 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. I fully agree. Actually, C is too crude on casts: there's just one almighty cast. C++ has split this in many casts (originally four, but there are more now, some of them being plain templated functions). We could use macros to write clearer casts, say *yyssp = NARROWING_CAST (yy_state_num, yystate); which could also, in debug mode, include bounds checking and magical pragmas to silence some compilers. But one has But in this precise case, I don't see much of a difference between a cast and dark pragma magic: in both cases it means "dear compiler, shut up", yet the cast is truly portable. > 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. Right. >> 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. I'll do that. Yet we need to find a way to have this be silenced. I tried also casting PTRDIFF_MAX to (ptrdiff_t), to no avail. Maybe the error in actually in the division by the unsigned and the diagnostic is wrong. I'll try that. Can't reproduce locally, it's on the CI only. > 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. Actually configure's --enable-gcc-warnings is also what selects the warnings for the test suite, for which I really want to have no warnings (because that's fewer complaints). Cheers!
