Some counterexamples seem to be repeated for -Wcounterexamples about bison HOT 7 OPEN

I'll check this out this weekend

from bison.

@Xaec6 Would you have a look at this issue?

from bison.

So what's happening here is the identical conflicts are coming from different states. FWIW, the 'warning: 4 shift/reduce conflicts' includes 1, 2, 4, 5. @akimd the conflicts counter is considering 2,3 and 5,6 to be the duplicates here because it doesn't differentiate between different s/r conflicts on the same token. I guess it's up to you which component should change its behavior so they match.

As for this duplicate, perhaps we change
cmDependsJavaParser.y: warning: shift/reduce conflict on token jp_DOT [-Wcounterexamples]
to
cmDependsJavaParser.y: warning: shift/reduce conflict on token jp_DOT in state 475 [-Wcounterexamples]
so it's more obvious these are different. While I could also add some logic to not report duplicates, it would frequently cause the number of counterexamples reported to be lower than the s/r conflict count which would be confusing.

from bison.

Thanks for looking into this - adding the state as you show looks good to me - couple suggestions to improve the output

1. report total and duplicate/unique counts
2. mark duplicate counterexamples somehow

On the face of it it looks like the duplicates are (2, 5) and (3, 6) - so I'm surprised you found (2, 3) and (5, 6) as duplicates

from bison.

@Xaec6 Thanks a lot for having looked at this into more details. I'm sorry I took so long to answer, but I think I needed a break.

I don't want to report state numbers in the diagnostics, IMHO that's really something that make only sense in the reports. I do agree that this situation is annoying though, and I definitely would like the "duplicate" counterexamples to not be displayed (I don't think it helps to see it several times. After all it is the same set of rules that have the same problem, but in some "duplicate subautomaton"). Maybe we should report when a counterexample applies for several conflicts, something like

cmDependsJavaParser.y: warning: shift/reduce conflict on token jp_DOT (in 2 states) [-Wcounterexamples]
Example: jp_THIS • jp_DOT Identifier
[...]

or (I prefer this one):

cmDependsJavaParser.y: warning: 2 shift/reduce conflicts on token jp_DOT [-Wcounterexamples]
Example: jp_THIS • jp_DOT Identifier
[...]

I had never realized (or I forgot) that we were counting multiple S/R conflicts in the same place as a single one. It would definitely make sense to count each conflict, but I'm afraid we can't change that, given that some people might be relying on this fact in their %expect requirements. That would break compatibility. I have no idea if some people do indeed rely on the current situation though.

We might have a way out by implementing both ways of counting, and enabling the new one only when %require "3.8" (or better) is enabled. And in a distant future, we might phase out the old way to counting.

from bison.

Hi @zmajeed

On the face of it it looks like the duplicates are (2, 5) and (3, 6) - so I'm surprised you found (2, 3) and (5, 6) as duplicates

Vincent is referring to something else. 2 and 5 are indeed "twice the same counterexample", but 2 and 3 are actually two SR conflicts in the same state-and-lookahead, and are counted as a single contribution to the overall number of conflicts. That's why the report is about "4 shift/reduce conflicts", and not 6.

I'm not sure what the cutest grammar to show this phenomenon would be, but the following one does it:

$ cat foo.y
%%
exp: a 'x' | 'x'
a: 'x' |
  
$ LC_ALL=C ∫ foo.y -Wcex
foo.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
foo.y: warning: shift/reduce conflict on token 'x' [-Wcounterexamples]
  Example: . 'x'
  Shift derivation
    exp
    `-> . 'x'
  Example: . 'x'
  Reduce derivation
    exp
    `-> a     'x'
        `-> .
foo.y: warning: shift/reduce conflict on token 'x' [-Wcounterexamples]
  First example: 'x'
  Shift derivation
    exp
    `-> 'x'
  Second example: 'x'
  Reduce derivation
    exp
    `-> 'x'
foo.y:3.9: warning: rule useless in parser due to conflicts [-Wother]
    3 | a: 'x' |
      |         ^

from bison.

Thanks for elaborating Akim - I think I understand the difference between duplicate counterexamples and two SR conflicts being counted as one - I'll need to go back to the states report to really figure this out

As for the changes you propose I too prefer the second suggestion "2 shift/reduce conflicts on token" - I can see why state numbers should not be referenced outside of the report itself since there's nothing tying the counterexamples output to the states report - also this format calls out duplicate counterexamples - and the multiplicity allows both unique and duplicate counts to be calculated

from bison.