This reverts commit d0bec3175f (which
should have read "We have a clash...", not "With have a clash...").
Now that `max()` was renamed `max_int()`, we can use `max` again, as
elsewhere in the code.
* src/counterexample.c (visited_hasher): Alpha reconversion.
Reported by Maarten De Braekeleer.
https://lists.gnu.org/r/bison-patches/2020-07/msg00080.html
We don't want to use gnulib's min and max macros, since we use
function calls in min/max arguments.
* src/location.c (max_int, min_int): Move to...
* src/system.h: here.
* src/counterexample.c, src/derivation.c: Use max_int instead of max.
With have a clash with the "max" function.
src/counterexample.c: In function 'visited_hasher':
src/counterexample.c:720:48: error: declaration of 'max' shadows a global declaration [-Werror=shadow]
src/counterexample.c:116:12: error: shadowed declaration is here [-Werror=shadow]
* src/counterexample.c (visited_hasher): Alpha conversion.
From
input.y: warning: reduce/reduce conflict on token $end [-Wcounterexamples]
Example: A b .
First derivation
a
`-> A b .
Second derivation
a
`-> A b
`-> b .
to
input.y: warning: reduce/reduce conflict on token $end [-Wcounterexamples]
Example: A b .
First reduce derivation
a
`-> A b .
Second reduce derivation
a
`-> A b
`-> b .
* src/counterexample.c (print_counterexample): here.
Compute the width of the labels to properly align the values.
* tests/conflicts.at, tests/counterexample.at, tests/diagnostics.at,
* tests/report.at: Adjust.
Now that the derivation is no longer printed on one line, aligning the
example and the derivation is no longer useful. It can actually be
harmful, as it makes the overall structure less clear.
* src/derivation.h, src/derivation.c (derivation_print_leaves): Remove
the `prefix` argument.
* src/counterexample.c (print_counterexample): Put the example next to
its label.
* tests/conflicts.at, tests/counterexample.at, tests/diagnostics.at,
* tests/report.at: Adjust.
Now that we use complain, the "sections" are clearer.
* src/counterexample.c (print_counterexample): Use the empty line only
in reports.
* tests/counterexample.at, tests/diagnostics.at, tests/report.at: Adjust.
This is more consistent, and brings benefits: users know that these
diagnostics are attached to -Wcounterexamples, and they can also click
on the hyperlink if permitted by their terminal.
We go from
warning: 1 reduce/reduce conflict [-Wconflicts-rr]
Reduce/reduce conflict on token $end:
Example A b .
First derivation a -> [ A b . ]
Second derivation a -> [ A b -> [ b . ] ]
to
warning: 1 reduce/reduce conflict [-Wconflicts-rr]
input.y: warning: reduce/reduce conflict on token $end [-Wcounterexamples]
Example A b .
First derivation a -> [ A b . ]
Second derivation a -> [ A b -> [ b . ] ]
with an hyperlink on -Wcounterexamples.
* src/counterexample.c (counterexample_report_reduce_reduce):
Use complain.
* tests/counterexample.at, tests/diagnostics.at, tests/report.at:
Adjust.
Sometimes, understanding the derivations is difficult, because they
are serialized to fit in one line. For instance, the example taken
from the NEWS file:
%token ID
%%
s: a ID
a: expr
expr: expr ID ',' | "expr"
gave
First example expr • ID ',' ID $end
Shift derivation $accept → [ s → [ a → [ expr → [ expr • ID ',' ] ] ID ] $end ]
Second example expr • ID $end
Reduce derivation $accept → [ s → [ a → [ expr • ] ID ] $end ]
Printing as trees, it gives:
First example expr • ID ',' ID $end
Shift derivation
$accept
↳ s $end
↳ a ID
↳ expr
↳ expr • ID ','
Second example expr • ID $end
Reduce derivation
$accept
↳ s $end
↳ a ID
↳ expr •
* src/glyphs.h, src/glyphs.c (down_arrow, empty, derivation_separator):
New.
* src/derivation.c (derivation_print, derivation_print_impl): Rename
as...
(derivation_print_flat, derivation_print_flat_impl): These.
(fputs_if, derivation_depth, derivation_width, derivation_print_tree)
(derivation_print_tree_impl, derivation_print): New.
* src/counterexample.c (print_counterexample): Adjust.
* tests/conflicts.at, tests/counterexample.at, tests/diagnostics.at,
* tests/report.at: Adjust.
When reporting counterexamples for s/r conflicts, put the shift first.
This is more natural, and displays the default resolution first, which
is also what happens for r/r conflicts where the smallest rule number
is displayed first, and "wins".
* src/counterexample.c (counterexample): Add a shift_reduce member.
(new_counterexample): Adjust.
Swap the derivations when this is a s/r conflict.
(print_counterexample): For s/r conflicts, prefer "Shift derivation"
and "Reduce derivation" rather than "First/Second derivation".
* tests/conflicts.at, tests/counterexample.at, tests/report.at: Adjust.
* NEWS, doc/bison.texi: Ditto.
Currently we use both names. Let's stick to the short one.
* src/AnnotationList.c, src/conflicts.c, src/counterexample.c,
* src/getargs.c, src/getargs.h, src/graphviz.c, src/ielr.c,
* src/lalr.c, src/print-graph.c, src/print-xml.c, src/print.c,
* src/state-item.c, src/state.c, src/state.h, src/tables.c:
s/lookahead_token/lookahead/gi.
* src/counterexample.c, src/parse-simulation.c: It is more usual in
Bison to use sizeof on expressions than on types, especially for
allocation.
Let the compiler do it's job instead of calling memcpy ourselves.
There are too many gl_list_t in there, it's hard to understand what is
going on. Introduce and use more precise types. I sure can be wrong
in some places, it's hard to tell without proper tool support.
* src/counterexample.c, src/lssi.c, src/lssi.h, src/parse-simulation.c,
* src/parse-simulation.h, src/state-item.c, src/state-item.h
(si_bfs_node_list, search_state_list, ssb_list, lssi_list)
(state_item_list): New.
Prefer `&foos[i]` to `foos + i` when `foos` is an array. IMHO, it
makes the semantics clearer.
* src/counterexample.c, src/lssi.c, src/parse-simulation.c,
* src/state-item.c: With arrays, prefer the array notation rather than
the pointer one.
It makes no sense, and is actually confusing, to display twice the
same example with no visible difference.
* src/complain.h, src/complain.c (is_styled): New.
* src/counterexample.c (print_counterexample): Display the unified
example a second time only if it makes a difference.
* tests/conflicts.at, tests/counterexample.at, tests/report.at: Adjust.
* tests/diagnostics.at: Make sure we do display the unifying examples
twice when colors are enabled. And check those colors.
Instead of
Shift/reduce conflict on token D:
Example A a • D
First derivation s ::=[ A a a ::=[ b ::=[ c ::=[ • ] ] ] d ::=[ D ] ]
Example A a • D
Second derivation s ::=[ A a d ::=[ • D ] ]
display
Shift/reduce conflict on token D:
Example A a • D
First derivation s ::=[ A a a ::=[ b ::=[ c ::=[ • ] ] ] d ::=[ D ] ]
Example A a • D
Second derivation s ::=[ A a d ::=[ • D ] ]
* src/counterexample.c (print_counterexample): Indent.
* tests/counterexample.at: Adjust.
Showing the items (with the state numbers) is really something we
should restrict to the report.
* src/counterexample.c (counterexample_report_shift_reduce)
(counterexample_report_reduce_reduce): Don't show the pointed rules,
we will do that in the report.
* tests/counterexample.at: Adjust.
* src/conflicts.c (find_state_item_number, report_state_counterexamples):
Move to...
* src/counterexample.h, src/counterexample.c (find_state_item_number)
(counterexample_report_state): this.
Add support for `out` as an argument.
(counterexample_report_reduce_reduce, counterexample_report_shift_reduce):
Accept an `out` argument, and be static.
Plural vs. singular is always a problem...
But we already have conflicts-sr and conflicts-rr, so counterexamples
makes more sense than counterexample. Besides, -Wcounterexample will
still be accepted as an unambiguous prefix of -Wcounterexamples.
Add -Wcex as a convenient alias.
While at it, use only "counterexample", never "counter example".
* src/complain.h, src/complain.c
(Wcounterexample, warning_counterexample): Rename as...
(Wcounterexamples, warning_counterexamples): these.
(argmatch_warning_docs): Rename -Wcounterexample as -Wcounterexamples.
(argmatch_warning_args): Likewise.
Add support for -Wcex.
Adjust dependencies.
Use colors to show the counterexamples and the derivations in color,
to highlight their structure. Align the outputs, and add i18n
support. Reduce width by using a one-space separator instead of
two-space.
From
Example A • B C
First derivation s ::=[ a ::=[ A • ] x ::=[ B C ] ]
Second derivation s ::=[ y ::=[ A • B ] c ::=[ C ] ]
to
Example A • B C
First derivation s ::=[ a ::=[ A • ] x ::=[ B C ] ]
Example A • B C
Second derivation s ::=[ y ::=[ A • B ] c ::=[ C ] ]
with colors.
* data/bison-default.css (cex-dot, cex-0, cex-1, cex-2, cex-3, cex-4)
(cex-5, cex-6, cex-7, cex-step, cex-leaf): New.
* src/derivation.c (derivation_print_styled_impl): New.
(derivation_print, derivation_print_leaves): Use it.
* src/counterexample.c: Reformat the output.
* tests/counterexample.at: Adjust.
In Bison we refer to "shift/reduce" conflicts, not "shift-reduce" (in
Bison 3.6.3 186 occurrences vs 15). Enforce consistency on this.
Instead of "spending" a second line for each conflict to report the
lookaheads, put that on the same line as the type of conflict. Also,
prefer "token" to "symbol". Maybe we should even prefer "lookahead".
While at it, enable internationalization, with plurals where
appropriate.
As a consequence, instead of
Shift-Reduce Conflict:
6: 3 b: . %empty
6: 6 d: c . A
On Symbol: A
display
Shift/reduce conflict on token A:
6: 3 b: . %empty
6: 6 d: c . A
* NEWS, doc/bison.texi, src/conflicts.c: Spell it "shift/reduce", not
"shift-reduce".
* src/counterexample.c (counterexample_report_shift_reduce)
(counterexample_report_reduce_reduce): Reformat and internationalize
output.
* tests/counterexample.at: Adjust expectations.
First, we should avoid code such as
gl_list_iterator_t it = gl_list_iterator (deriv->children);
derivation *child = NULL;
while (gl_list_iterator_next (&it, (const void **) &child, NULL))
{
derivation_print (child, f);
because of -Wstrict-aliasing (whose job is to catch type-punning
issues). See https://lists.gnu.org/r/bug-bison/2020-05/msg00039.html.
Rather we need
gl_list_iterator_t it = gl_list_iterator (deriv->children);
const void **p = NULL;
while (gl_list_iterator_next (&it, &p, NULL))
{
derivation *child = (derivation *) p;
derivation_print (child, f);
Second, list iterators actually have destructors. Even though they
are noop in the case of linked-lists, we should use them.
Let's address both issues with typed wrappers (such as
derivation_list_next) that take care of both issues, and besides allow
to scope the iterators within the loop:
derivation *child;
for (gl_list_iterator_t it = gl_list_iterator (deriv->children);
derivation_list_next (&it, &child);
)
{
derivation_print (child, f);
* src/derivation.h, src/derivation.c (derivation_list_next): New.
Use it where appropriate.
* src/counterexample.c (search_state_list_next): New.
Use it where appropriate.
* src/parse-simulation.h, src/parse-simulation.c
* src/state-item.h (state_item_list_next): New.
Use it where appropriate.
Instead of `On Symbols: {b,c,}`, display `On Symbols: b, c`.
* src/counterexample.c (counterexample_report_reduce_reduce): We don't
need braces.
Use commas as a separator, not a terminator.
* tests/counterexample.at: Adjust.
* src/parse_simulation.c: Fix bug in parse_state_free.
Free new_root when simulate_reduction generates zero states.
* src/parse-simulation.c, src/parse-simulation.h
(parse_state_list, parse_state_list_append): New.
* src/parse-simulation.c, src/parse-simulation.h,
* src/counterexample.c: Replace all uses of lists of parse states and
appends to parse_state_lists with the new API.
* src/derivation.h, src/derivation.c: Make derivation struct opaque.
Add derivation_list type for clarity.
(derivation_list_new): New.
(derivation_list_append): New.
(derivation_list_prepend): New.
(derivation_new_leaf): New constructor for derivations with no
children.
* src/counterexample.c, src/parse-simulation.c,
* src/parse-simulation.h: Replace uses of gl_list_t containing
derivations with derivation_list and its API.
Replace calls of dervation_new using null children with
derivation_new_leaf.
* src/parse-simulation.c: replace ps_chunk and its API with typed
versions si_chunk and deriv_chunk.
* src/parse-simlation.h, src/parse-simulation.c: Remove
parse_state_retain_deriv in favor of derivation reference counting.
* src/counterexample.c: Remove search_state_retain_deriv.
* src/counterexample.c: Use 'res' as a variable name for returned
value, as elsewhere.
Avoid uninitialized variables, especially pointers.
Avoid assignment where possible.
