[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: |
Akim Demaille |
Subject: |
Re: [PATCH 0/3] yacc: compute the best type for the state number |
Date: |
Sat, 5 Oct 2019 12:07:34 +0200 |
Hi Paul,
Thanks for the quick response!
> Le 5 oct. 2019 à 10:48, Paul Eggert <address@hidden> 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!
- Re: [PATCH 0/3] yacc: compute the best type for the state number, (continued)
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/03
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/03
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/03
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/04
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/06
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/06
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number,
Akim Demaille <=
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Akim Demaille, 2019/10/05
- Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
Re: [PATCH 0/3] yacc: compute the best type for the state number, Paul Eggert, 2019/10/05
Re: [PATCH 0/3] yacc: compute the best type for the state number, Théophile Ranquet, 2019/10/25