Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Printer for nonempty lists: "pattern matching is not exhaustive" warning in ocaml backend (suppressed in Haskell backend) #384

Closed
JoAllg opened this issue Sep 21, 2021 · 7 comments · Fixed by #385
Assignees
Labels
Haskell lists Concerning list categories and separator/terminator/delimiter pragmas OCaml printer Concerning the generated printer
Milestone

Comments

@JoAllg
Copy link

JoAllg commented Sep 21, 2021

Background: I want to have a list of Data category elements with at least two elements. The easiest working solution I found is

DPair. Data ::= "Pair" Pair ;
DPairSeq1.   Pair ::= Data Data ;
DPairSeq2.   Pair ::= Data Pair ;

However, here are two other ways i thought of, that cause a "pattern matching is not exhaustive" warning in the ocaml / ocaml-menhir backend:

DPair. Data ::= "Pair" Pair ;
terminator nonempty Pair "" ;
-- DPairSeq1.   Pair ::= Data Data ;
DPairSeq2. Pair ::= Data [Pair] ;

The full error message:

...
ocamllex LexMichelson.mll
28 states, 718 transitions, table size 3040 bytes
ocamlc  -o TestTest BNFC_Util.ml AbsTest.ml SkelTest.ml ShowTest.ml PrintTest.ml ParTest.mli ParTest.ml LexTest.ml TestTest.ml
File "PrintTest.ml", lines 91-93, characters 33-63:
91 | .................................match (i, es) with
92 |     (_,[x]) -> (concatD [prtPair 0 x])
93 |   | (_,x::xs) -> (concatD [prtPair 0 x ; prtPairListBNFC 0 xs])
Warning 8: this pattern-matching is not exhaustive.
Here is an example of a case that is not matched:
(_, [])

The second gramar that causes the same error:

DPair. Data ::= "Pair" Data [Pair] ;
terminator nonempty Pair "" ;
DPairSeq2. Pair ::= Data ;

The warning does not occur if nonempty is removed.

@JoAllg JoAllg changed the title "not exhaustive pattern matching" warning in ocaml backend "pattern matching is not exhaustive" warning in ocaml backend Sep 21, 2021
@andreasabel
Copy link
Member

andreasabel commented Sep 21, 2021

terminator nonempty Pair "" ;
File "PrintTest.ml", lines 91-93, characters 33-63:
91 | .................................match (i, es) with
92 |     (_,[x]) -> (concatD [prtPair 0 x])
93 |   | (_,x::xs) -> (concatD [prtPair 0 x ; prtPairListBNFC 0 xs])
Warning 8: this pattern-matching is not exhaustive.
Here is an example of a case that is not matched:
(_, [])

Since Pair lists cannot be empty, the case for empty lists cannot occur when trees are printed that were generated by the parser. So this warning can be ignored.

In #371 it is suggested that nonempty lists should be also represented as such in the AST. Haskell by now has a type for non-empty lists in the base library. I do not know how the situation is in Ocaml. In any case, this would be backwards-incompatible, so I am hesitant to make such a change.

One could of course silence the Ocaml warning by adding a case for the empty list that creates the empty document. This could be beneficial if the programmer wants to create empty lists in the AST (only not via parsing).

However, if the programmer rather wants to insist that lists there can indeed not be empty, then it is cleaner to leave the match partial so that the program crashes if an empty list should be encountered. (Well, maybe you do not want to crash in the printer...)

@andreasabel andreasabel added lists Concerning list categories and separator/terminator/delimiter pragmas OCaml printer Concerning the generated printer labels Sep 21, 2021
@JoAllg
Copy link
Author

JoAllg commented Sep 21, 2021

Ah sorry I thought it was a problem with my certain case and not generally connected to nonempty and ocaml.

One could implement a nonempty list, but that seem overkill for me for the case of the printer.
May I suggest to add the case | [] -> failwith "Empty List"?
I think that would satisfy the consideration you explained last and still allow for manuall modification for the other case.

@andreasabel
Copy link
Member

The Haskell backend also creates an incomplete match, but suppresses the warning.
The imperative backends (C, C++, Java) simply print nothing when the list is empty.
Maybe we should do the same in the functional backends.

@andreasabel andreasabel added this to the 2.9.3 milestone Sep 22, 2021
@andreasabel andreasabel changed the title "pattern matching is not exhaustive" warning in ocaml backend Printer for nonempty lists: "pattern matching is not exhaustive" warning in ocaml backend (suppressed in Haskell backend) Sep 22, 2021
@andreasabel
Copy link
Member

There is a related problem with lists of precedence categories:

terminator Exp2 "";
EInt.  Exp2 ::= Integer;

Generated printer in OCaml (same in Haskell):

87 | ................................match (i, es) with
88 |     (2,[]) -> (concatD [])
89 |   | (2,x::xs) -> (concatD [prtExp 2 x ; prtExpListBNFC 2 xs])
Warning 8: this pattern-matching is not exhaustive.
Here is an example of a case that is not matched:
(0, _)

This would be a bit harder to fix than the OP.

@andreasabel
Copy link
Member

PR #385 would make the OP go away, but not #384 (comment). So I am not sure whether to merge this. Alternatively, maybe the warning could be switched off in OCaml.

@andreasabel andreasabel self-assigned this Sep 22, 2021
@JoAllg
Copy link
Author

JoAllg commented Sep 22, 2021

As for disabling the warning in OCaml, to catch the case and throw an error seems to be the most straightforward way
| (_, [] ) -> failwith "Error Message"

However apparently it is also possible to disable the warning just for this part of the code by adding [@warning "-8"] ... [@warning "+8"] like that:

(generated Printer in Ocaml)

and [@warning "-8"] prtPPairListBNFC  i  es  : doc [@warning "+8"] = match (i, es)  with
    (_,[x]) -> (concatD [prtPPair 0 x])
  | (_,x::xs) -> (concatD [prtPPair 0 x ; prtPPairListBNFC 0 xs])

(cf. https://stackoverflow.com/questions/46489249/use-ocaml-warning-attribute-to-disable-warning-8-unexhaustive-match )

andreasabel added a commit that referenced this issue Sep 22, 2021
…ists

We make the lowest precedence the catch-all.
@andreasabel
Copy link
Member

Thanks for the research @JoAllg!

I am now also fixing the problem with the precedence category lists, so the pattern matches should be complete now for both OCaml and Haskell.

andreasabel added a commit that referenced this issue Sep 22, 2021
…ists

We make the lowest precedence the catch-all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Haskell lists Concerning list categories and separator/terminator/delimiter pragmas OCaml printer Concerning the generated printer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants