reduce/reduce conflicts are bad, especially so since they're undocumented. We don't know whether this introduced parser bugs or not. Mike - could you look at this please? It was your commit that introduced the new conflicts. Cheers, Simon On 02/12/2014 10:19, Dr. ERDI Gergo wrote:

On Mon, 1 Dec 2014, Richard Eisenberg wrote:

...

In unrelated work, I saw this scroll across when happy'ing the parser:

...

shift/reduce conflicts: 60 reduce/reduce conflicts: 16

These numbers seem quite a bit higher than what I last remember (which is something like 48 and 1, not 60 and 16). Does anyone know why?

The offending commit is bc2289e13d9586be087bd8136943dc35a0130c88. I know this because I was changing the parser for patsyn signatures, and so I updated the numbers in Parser.y to make sure I'm not adding any new conflicts:

25 June 2014

Conflicts: 47 shift/reduce 1 reduce/reduce

but then when time came to rebase my changes before pushing, I noticed that it has gone up, and I had to update it yet again in Parser.y:

20 Nov 2014

Conflicts: 60 shift/reduce 12 reduce/reduce

So anyway, the point is, if you try bc2289e and bc2289e^ you can see that that is the commit that introduced these new conflicts. _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

On Wed, 03 Dec 2014 11:59:42 +0000 Simon Marlow wrote:

...

...

In unrelated work, I saw this scroll across when happy'ing the parser:

...

shift/reduce conflicts: 60 reduce/reduce conflicts: 16

These numbers seem quite a bit higher than what I last remember (which is something like 48 and 1, not 60 and 16). Does anyone know why?

4 of reduce/reduce conflicts are result of exact rule copy: https://phabricator.haskell.org/D569

reduce/reduce conflicts are bad, especially so since they're undocumented. We don't know whether this introduced parser bugs or not. Mike - could you look at this please? It was your commit that introduced the new conflicts.

Agreed. 11 more reduce/reduce (of left 12) came from single scary rule added in

commit bc2289e13d9586be087bd8136943dc35a0130c88 ghc generates more user-friendly error messages

exp10 :: { LHsExpr RdrName } ... | 'let' binds {% parseErrorSDoc (combineLocs $1 $2) $ text "parse error in let binding: missing required 'in'" } The other rules add shift/reduce conflicts as follows: -- parsing error messages go below here {- s/r:1 r/r:0 -} | '\\' apat apats opt_asig '->' {% parseErrorSDoc (combineLocs $1 $5) $ text "parse error in lambda: no expression after '->'" {- s/r:1 r/r:0 -} | '\\' {% parseErrorSDoc (getLoc $1) $ text "parse error: naked lambda expression '\'" } {- s/r:1 r/r:0 -} | 'let' binds 'in' {% parseErrorSDoc (combineLocs $1 $2) $ text "parse error in let binding: missing expression after 'in'" } {- s/r:0 r/r:11 -} | 'let' binds {% parseErrorSDoc (combineLocs $1 $2) $ text "parse error in let binding: missing required 'in'" } {- s/r:0 r/r:0 -} | 'let' {% parseErrorSDoc (getLoc $1) $ text "parse error: naked let binding" } {- s/r:1 r/r:0 -} | 'if' exp optSemi 'then' exp optSemi 'else' {% hintIf (combineLocs $1 $5) "else clause empty" } {- s/r:2 r/r:0 -} | 'if' exp optSemi 'then' exp optSemi {% hintIf (combineLocs $1 $5) "missing required else clause" } {- s/r:1 r/r:0 -} | 'if' exp optSemi 'then' {% hintIf (combineLocs $1 $2) "then clause empty" } {- s/r:2 r/r:0 -} | 'if' exp optSemi {% hintIf (combineLocs $1 $2) "missing required then and else clauses" {- s/r:2 r/r:0 -} | 'if' {% hintIf (getLoc $1) "naked if statement" } {- s/r:0 r/r:0 -} | 'case' exp 'of' {% parseErrorSDoc (combineLocs $1 $2) $ text "parse error in case statement: missing list after '->'" } {- s/r:1 r/r:0 -} | 'case' exp {% parseErrorSDoc (combineLocs $1 $2) $ text "parse error in case statement: missing required 'of'" } {- s/r:1 r/r:0 -} | 'case' {% parseErrorSDoc (getLoc $1) $ text "parse error: naked case statement" } Shift/reduces look harmless (like MultiWayIf ambiguity) as they seem to resolve as shift correctly. -- Sergei

It would be great to have a patch that documents all this Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Sergei Trofimovich | Sent: 13 December 2014 15:20 | To: GHC Devs | Cc: Simon Marlow; Dr. ERDI Gergo; Mike Izbicki | Subject: Re: more parser conflicts? | | On Wed, 03 Dec 2014 11:59:42 +0000 | Simon Marlow wrote: | | > >> In unrelated work, I saw this scroll across when happy'ing the | parser: | > >> | > >>> shift/reduce conflicts: 60 | > >>> reduce/reduce conflicts: 16 | > >> | > >> These numbers seem quite a bit higher than what I last remember | > >> (which is something like 48 and 1, not 60 and 16). Does anyone | know why? | | 4 of reduce/reduce conflicts are result of exact rule copy: | https://phabricator.haskell.org/D569 | | > reduce/reduce conflicts are bad, especially so since they're | > undocumented. We don't know whether this introduced parser bugs or | not. | > Mike - could you look at this please? It was your commit that | > introduced the new conflicts. | | Agreed. | | 11 more reduce/reduce (of left 12) came from single scary rule added | in | > commit bc2289e13d9586be087bd8136943dc35a0130c88 | > ghc generates more user-friendly error messages | | exp10 :: { LHsExpr RdrName } | ... | | 'let' binds {% parseErrorSDoc (combineLocs $1 $2) $ text | "parse error in let binding: missing | required 'in'" | } | | The other rules add shift/reduce conflicts as follows: | | -- parsing error messages go below here | {- s/r:1 r/r:0 -} | | '\\' apat apats opt_asig '->' {% parseErrorSDoc (combineLocs $1 | $5) $ text | "parse error in | lambda: no expression after '->'" | {- s/r:1 r/r:0 -} | | '\\' {% parseErrorSDoc | (getLoc $1) $ text | "parse error: | naked lambda expression '\'" | } | {- s/r:1 r/r:0 -} | | 'let' binds 'in' {% parseErrorSDoc | (combineLocs $1 $2) $ text | "parse error in | let binding: missing expression after 'in'" | } | {- s/r:0 r/r:11 -} | | 'let' binds {% parseErrorSDoc | (combineLocs $1 $2) $ text | "parse error | in let binding: missing required 'in'" | } | {- s/r:0 r/r:0 -} | | 'let' {% parseErrorSDoc | (getLoc $1) $ text | "parse error: | naked let binding" | } | {- s/r:1 r/r:0 -} | | 'if' exp optSemi 'then' exp optSemi 'else' {% hintIf | (combineLocs $1 $5) "else clause empty" } | {- s/r:2 r/r:0 -} | | 'if' exp optSemi 'then' exp optSemi {% hintIf | (combineLocs $1 $5) "missing required else clause" } | {- s/r:1 r/r:0 -} | | 'if' exp optSemi 'then' {% hintIf | (combineLocs $1 $2) "then clause empty" } | {- s/r:2 r/r:0 -} | | 'if' exp optSemi {% hintIf | (combineLocs $1 $2) "missing required then and else clauses" | {- s/r:2 r/r:0 -} | | 'if' {% hintIf (getLoc | $1) "naked if statement" } | {- s/r:0 r/r:0 -} | | 'case' exp 'of' {% parseErrorSDoc | (combineLocs $1 $2) $ text | "parse error | in case statement: missing list after '->'" | } | {- s/r:1 r/r:0 -} | | 'case' exp {% parseErrorSDoc | (combineLocs $1 $2) $ text | "parse error in | case statement: missing required 'of'" | } | {- s/r:1 r/r:0 -} | | 'case' {% parseErrorSDoc | (getLoc $1) $ text | "parse error: | naked case statement" | } | | Shift/reduces look harmless (like MultiWayIf ambiguity) as they seem | to resolve as shift correctly. | | -- | | Sergei