Let's make a difference between places where Perl is required for the
test (AT_PERL_REQUIRE), and the places where it's used to run the
test, but it's not not to run the test (AT_PERL_CHECK).
* tests/local.at (AT_REQUIRE): New.
(AT_PERL_CHECK, AT_PERL_REQUIRE): New.
Use them where appropriate.
* tests/local.mk ($(TESTSUITE)): Beware not to start the line with
'-pi' if Perl is empty, as Make understands this as "it's ok to fail".
Which it is not.
My previous tests (with ./configure PERL=false) have been fooled by
configure, that managed to find perl anyway. This time, I ran this on
a Fedora in Docker, without Perl.
* tests/calc.at, tests/diagnostics.at, tests/headers.at,
* tests/input.at, tests/local.at, tests/named-refs.at,
* tests/output.at, tests/regression.at, tests/skeletons.at,
* tests/synclines.at, tests/torture.at: Don't require Perl.
* data/skeletons/c.m4 (b4_c99_int_type_define): Reorder to put the
signed types first, since they’re simpler and this keeps similar
code closer. For signed types, don’t bother checking whether the
type promotes to int since the type must be signed anyway. For
unsigned types, protect a test like ‘UCHAR_MAX <= INT_MAX’ with
‘!defined __UINT_LEAST8_MAX__’, as otherwise the logic is wrong
for oddball platforms; and once we do that, there should no need
for ‘defined INT_MAX’ so remove that.
Currently we face test suite failures in different environments,
because of a conflict between the definitions of isnan by gnulib, and
by the C++ library:
262. headers.at:186: testing Sane headers: %locations %debug c++ ...
./headers.at:186: COLUMNS=1000; export COLUMNS; bison --color=no -fno-caret -d -o input.cc input.y
./headers.at:186: $CXX $CXXFLAGS $CPPFLAGS -c -o input.o input.cc
stderr:
In file included from /usr/include/c++/4.8.2/cmath:44:0,
from /usr/include/c++/4.8.2/random:38,
from /usr/include/c++/4.8.2/bits/stl_algo.h:65,
from /usr/include/c++/4.8.2/algorithm:62,
from location.hh:41,
from input.hh:90,
from input.cc:50:
/u/cs/fac/eggert/src/gnu/bison/lib/math.h: In function 'bool isnan(double)':
/u/cs/fac/eggert/src/gnu/bison/lib/math.h:2849:1: error: new declaration 'bool isnan(double)'
_GL_MATH_CXX_REAL_FLOATING_DECL_2 (isnan, isnan, bool)
^
In file included from /usr/include/features.h:375:0,
from /usr/include/c++/4.8.2/x86_64-redhat-linux/bits/os_defines.h:39,
from /usr/include/c++/4.8.2/x86_64-redhat-linux/bits/c++config.h:2097,
from /usr/include/c++/4.8.2/cstdlib:41,
from input.hh:48,
from input.cc:50:
/usr/include/bits/mathcalls.h:235:1: error: ambiguates old declaration 'int isnan(double)'
__MATHDECL_1 (int,isnan,, (_Mdouble_ __value)) __attribute__ ((__const__));
^
There might be something to do in gnulib about this, but I believe
that gnulib should not be used in the test suite in the first place.
The test suite should work with other compilers than the one used to
compile the package. For a start, Bison sources are more
demanding (C99) than the generated parsers. Last time I tried, tcc
for example, was not able to compile Bison, yet our generated parsers
should compile cleanly with it.
Besides the problem at hand is with the C++ compiler, with is not the
one used to set up gnulib at configuration-time (config.h is mainly
built from probing the C compiler).
We should really not depend on gnulib in tests.
This was introduced in 2001 to check whether including
stdlib.h/string.h is safe thanks to STDC_HEADERS
(2ce1014469). Today, we assume at least
a C90 compiler, it should be safe enough.
* tests/local.at, tests/testsuite.h: Do not include config.h.
* tests/atlocal.in (conftest.cc): Likewise.
(CPPFLAGS): Do not expose lib/, as because of this we might picked up
gnulib replacement headers for system headers.
* tests/input.at: Use int instead of ptrdiff_t, for easier portability
(some machine on the CI did not find ptrdiff_t).
* tests/c++.at: Add missing include for getchar.
A number of portability issues with GCC 4.6 .. 4.9 (inclusive):
input.c:184:7: error: "UCHAR_MAX" is not defined [-Werror=undef]
#elif UCHAR_MAX <= INT_MAX
^
input.c:184:20: error: "INT_MAX" is not defined [-Werror=undef]
#elif UCHAR_MAX <= INT_MAX
^
input.c:202:7: error: "USHRT_MAX" is not defined [-Werror=undef]
#elif USHRT_MAX <= INT_MAX
^
input.c:202:20: error: "INT_MAX" is not defined [-Werror=undef]
#elif USHRT_MAX <= INT_MAX
^
* data/skeletons/c.m4 (b4_c99_int_type_define): Don't assume they are
defined.
Flex should not be required to build Bison or run the test suite (of
course it is needed for maintaining Bison). Yet the Automake
conditional FLEX_WORKS does not work.
* m4/flex.m4 (_AC_PROG_LEX_YYTEXT_DECL): Since this is called
conditionally, don't define LEX_IS_FLEX here, but rather...
(AC_PROG_LEX): here.
* configure.ac: Be more cautious about possibly undefined variables.
That way, glr.c can use it too.
* data/skeletons/c.m4 (b4_int_type):
Do not special-case ‘char’; it’s not worth the trouble,
as clang complains about char subscripts.
(b4_c99_int_type, b4_c99_int_type_define): New macros,
taken from yacc.c.
* data/skeletons/glr.c: Use b4_int_type_define.
* data/skeletons/yacc.c (b4_int_type): Remove, since there’s
no longer any need to redefine it.
Use b4_c99_int_type_define rather than its body.
This changes the Yacc skeleton to use “least” integer types to
keep tables smaller on some platforms, which should lessen cache
pressure. Since Bison uses the Yacc skeleton, it follows suit.
* data/skeletons/yacc.c: Include limits.h and stdint.h if this
seems to be needed.
(yytype_uint8, yytype_int8, yytype_uint16, yytype_int16):
If available, use GCC predefined macros __INT_MAX__ etc. to select
a “least” type, as this avoids namespace hassles. Otherwise, if
available fall back on selecting a “least” type via the C99 macros
INT_MAX, INT_LEAST8_MAX, etc. Otherwise, fall further back on one of
the builtin C99 types signed char, short, and int. Make sure that
any selected type promotes to int. Ignore any macros YYTYPE_INT16,
YYTYPE_INT8, YYTYPE_UINT16, YYTYPE_UINT8 defined by the user.
(ptrdiff_t, PTRDIFF_MAX): Simplify in the light of the above.
(yytype_uint8, yytype_uint16): Do not assume that unsigned char
and unsigned short promote to int, as this isn’t true on some
platforms (e.g., TI TMS320C55x).
* src/parse-gram.y (YYTYPE_INT16, YYTYPE_INT8, YYTYPE_UINT16)
(YYTYPE_UINT8): Remove, as these are no longer effective.
* data/skeletons/yacc.c (YYPTRDIFF_T, YYPTRDIFF_MAXIMUM):
Default to long, not int.
(yy_lac_stack_realloc, yy_lac, yytnamerr, yyparse):
Avoid casts to YYPTRDIFF_T that were masking the problem.
Because the checking of the grammar is made by phases after the whole
grammar was read, we sometimes have diagnostics that look weird. In
some case, within one type of checking, the entities are not checked
in the order in which they appear in the file. For instance, checking
symbols is done on the list of symbols sorted by tag:
foo.y:1.20-22: warning: symbol BAR is used, but is not defined as a token and has no rules [-Wother]
1 | %destructor {} QUX BAR
| ^~~
foo.y:1.16-18: warning: symbol QUX is used, but is not defined as a token and has no rules [-Wother]
1 | %destructor {} QUX BAR
| ^~~
Let's sort them by location instead:
foo.y:1.16-18: warning: symbol 'QUX' is used, but is not defined as a token and has no rules [-Wother]
1 | %destructor {} QUX BAR
| ^~~
foo.y:1.20-22: warning: symbol 'BAR' is used, but is not defined as a token and has no rules [-Wother]
1 | %destructor {} QUX BAR
| ^~~
* src/location.h (location_cmp): Be robust to empty file names.
* src/symtab.c (symbol_cmp): Sort by location.
* tests/input.at: Adjust expectations.
From
input.y:1.17-19: warning: symbol baz is used, but is not defined as a token and has no rules [-Wother]
1 | %printer {} foo baz
| ^~~
to
input.y:1.17-19: warning: symbol 'baz' is used, but is not defined as a token and has no rules; did you mean 'bar'? [-Wother]
1 | %printer {} foo baz
| ^~~
| bar
* bootstrap.conf: We need fstrcmp.
* src/symtab.c (symbol_from_uniqstr_fuzzy): New.
(complain_symbol_undeclared): Use it.
* tests/diagnostics.at (Suggestions): New.
* data/bison-default.css (insertion): Rename as...
(fixit-insert): this, as this is what GCC uses.
* src/symtab.c (complain_symbol_undeclared): New.
Use it.
Use quote on the guilty symbol (like GCC does, and we also do
elsewhere).
* tests/input.at: Adjust.
Both are stored in a hash, and back in the days, we used to iterate
over these tables using hash_do_for_each. However, the order of
traversal was not deterministic, which was a nuisance for
deterministic output (and therefore also a problem for tests). So at
some point (83b60c97ee) we generated a
sorted list of these symbols, and symbols_do actually iterated on that
list. But we kept the constraints of using hash_do_for_each, which
requires a lot of ceremonial code, and makes it hard/unnatural to
preserve data between iterations (see the next commit).
Alas, this is C, not C++.
Let's remove this abstraction, and directly iterate on the sorted
tables.
* src/symtab.c (symbols_do): Remove.
Adjust callers to use a simple for-loop instead.
(table_sort): New.
(symbols_check_defined): Use it.
(symbol_check_defined_processor, symbol_pack_processor)
(semantic_type_check_defined_processor, symbol_translation_processor):
Remove.
Simplify the corresponding functions (that no longer need to return a
bool).
This commit adds the suggestion in green, on the line below the
caret-and-tildes.
foo.y:1.1-14: warning: deprecated directive: '%error-verbose', use '%define parse.error verbose' [-Wdeprecated]
1 | %error-verbose
| ^~~~~~~~~~~~~~
| %define parse.error verbose
The current approach, with location_caret_suggestion, is fragile:
there's a protocol of calls to the complain functions which is strict.
We should rather have a richer structure describing the diagnostics,
including with submessages such as the suggestions, passed in the end
to the routines in charge of formatting and printing them.
* src/location.h, src/location.c (location_caret_suggestion): New.
* src/complain.c (deprecated_directive): Use it.
* tests/diagnostics.at, tests/input.at: Adjust expectations.
* src/location.c (caret_set_file): New.
Store the current line's length in caret_info.line_len.
Pay attention to fseek's return value.
Extracted from...
(location_caret): here.
I no longer agree with that item, there are indeed two things to
report: lack of definition, and being useless. We could have either
one without the other, they are not directly related.
input.c: In function 'int yyparse()':
input.c: error: conversion to 'long int' from 'long unsigned int'
may change the sign of the result [-Werror=sign-conversion]
yyes_capacity = sizeof yyesa / sizeof *yyes;
^
cc1plus: all warnings being treated as errors
* data/skeletons/yacc.c: here.
GCC 4.8 reports:
input.y:57:33: error: conversion to 'int' from 'long unsigned int'
may alter its value [-Werror=conversion]
int input_elts = sizeof input / sizeof input[0];
^
* tests/local.at (AT_YYLEX_DEFINE(c)): Add a cast (sorry, Paul!).
* src/location.c (caret_set_file):
* src/scan-code.l (contains_dot_or_dash):
Do not quietly convert pointer to bool, as Oracle Developer Studio
12.6 complains and it is arguably confusing style anyway.
