Skip to content

Fix the reduce/reduce conflicts in the grammar#599

Merged
lpw25 merged 2 commits intoocaml:trunkfrom
yallop:fix-local-open-pattern-grammar
Jun 15, 2016
Merged

Fix the reduce/reduce conflicts in the grammar#599
lpw25 merged 2 commits intoocaml:trunkfrom
yallop:fix-local-open-pattern-grammar

Conversation

@yallop
Copy link
Copy Markdown
Member

@yallop yallop commented May 31, 2016

This pull request fixes the conflicts in the grammar in the current trunk. The present status of parsing/parser.mly is as follows:

boot/ocamlyacc -v -e parsing/parser.mly
1 rule never reduced
126 reduce/reduce conflicts.

Happily, the conflicts all have straightforward resolutions. The changes are as follows:

  • The following productions are removed:

    pattern -> mod_longident DOT LPAREN pattern RPAREN
    pattern -> mod_longident DOT LPAREN pattern error
    pattern -> mod_longident DOT LPAREN error
    

    These productions are redundant because the RHSs can be generated via different routes:

    pattern ->
     pattern_gen ->
       simple_pattern ->
          simple_pattern_not_ident ->
             mod_longident DOT LPAREN pattern RPAREN
    

    (etc.)

  • The following duplication is removed:

    simple_delimited_pattern -> LBRACE lbl_pattern_list error
    simple_delimited_pattern -> LBRACE lbl_pattern_list error
    
  • The following redundancy

    simple_pattern_not_ident ->
      simple_delimited_pattern ->
         LBRACKET RBRACKET
    simple_pattern_not_ident ->
      constr_longident ->
         LBRACKET RBRACKET
    

    is replaced by removing the first production and adding a new production for qualified [] patterns:

    simple_pattern_not_ident -> mod_longident DOT LBRACKET RBRACKET
    

@Octachron
Copy link
Copy Markdown
Member

Sorry for introducing the conflicts in the first place. As a review data point, I have arrived independently to the same patch in https://github.com/Octachron/ocaml/tree/fix_parser_conflicts.

| mod_longident DOT simple_delimited_pattern
{ mkpat @@ Ppat_open(mkrhs $1 1, $3) }
| mod_longident DOT LBRACKET RBRACKET
{ mkpat @@ Ppat_construct ( mkrhs (Lident "[]") 1, None) }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a new syntax, right ? M.[] is not valid in patterns in 4.03.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it should probably Ppat_open the longident.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right M.[] should be the pattern open symmetric of M.[] ⇔ let open M in [] on the expression side. So, this is one difference between the two patch sets.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, you should wrap a Ppat_open around the Ppat_construct.

@Drup the syntax was introduced in trunk some time ago.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments. I intentionally kept the behaviour the same as trunk, where M is ignored in the pattern M.[]. But I agree that using M to resolve [] is better, so I've added Ppat_open in 1c4ee3a.

@damiendoligez damiendoligez added this to the 4.04 milestone Jun 7, 2016
@lpw25 lpw25 merged commit 712e395 into ocaml:trunk Jun 15, 2016
@yallop yallop deleted the fix-local-open-pattern-grammar branch June 15, 2016 14:26
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
Fix the reduce/reduce conflicts in the grammar
stedolan pushed a commit to stedolan/ocaml that referenced this pull request May 24, 2022
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants