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
Puzzling infinite loop with oddly accepted program #6327
Comments
Comment author: @chambart The problem is that a match case can be terminated with a semicolon, hence this is parsed as: let () = If you added parenthesis around ((); ; loop (n - 1)) you would have the expected parse error. A possible fix (like the ugly patch) is to forbid match (and anything that terminates with a match_case) as the first element of a sequence. I don't know if it breaks any existing code (at least not the compiler). |
Comment author: @dbuenzli Huhuhu, okay, makes sense. I guess this behaviour is to match what is done for ifs. if n = 0 then () else (); match n with 0 -> () | n -> (); I always thought you had to put parens around any match to make sequencing work. I don't think anything good can be done. I'll only mention that I almost got crazy during a whole hour staring a small loop where I had commented a single instruction but without its semi-colon which made it like this: let rec loop n = match n with And suddendly I had a very weird loop that whas printf'ing "stop" but would still continue... Oh well. |
Comment author: @dbuenzli Regarding your patch, maybe it should rather be turned into a warning ? |
Comment author: @protz Actually, that's an amazing feature! I'm always been writing: stmt1; begin match ... with | ... | ... end; stmt2 now I can do stmt1; match ... with | ... | ... ; ; stmt2 and I'm saving seven keystrokes! We should document this. |
Comment author: @damiendoligez This is a variant of #4627, which we decided not to fix. Shall we reverse that decision? |
Comment author: @dbuenzli I don't know. It happened to me once in 12 years of OCaml programing and I it took me time to realize because 1) I was very tired 2) the offending semi-colon was near the 80th column; I think in the loop example I give above it's more evident that this semi-colon may be suspicious when you get weird behaviour. It could still be an interesting experiment to try to make the change and test it with a switch against the opam repository for failures. |
Comment author: @damiendoligez Another possibility is to make the lexer emit a warning when it sees two single-semicolons in a row (not to be confused with a double-semicolon). Ad hoc, but simple, and it avoids touching the grammar (here be dragons). |
Comment author: @xavierleroy I'm not a fan of optional semicolons here and there, but since we've had them for nearly 20 years, we have to live with them and clench our teeth. I'm afraid no amounts of warnings will fix the problem. At any rate, I'm pushing this and #6961 to "later" than 4.03. |
I think this can be closed. |
Original bug ID: 6327
Reporter: @dbuenzli
Status: acknowledged (set by @mshinwell on 2014-05-30T11:46:11Z)
Resolution: open
Priority: low
Severity: tweak
Version: 4.01.0
Target version: later
Category: lexing and parsing
Tags: patch
Related to: #4627 #6961
Bug description
AFAIR my OCaml syntax this program should be rejected. It is accepted and running it (osx) results in an infinite loop.
let () =
let rec loop n = match n with
| 0 -> ()
| n -> (); ; loop (n - 1)
in
loop 3
real 0m15.264s
user 0m15.260s
sys 0m0.003s
Something must be wrong in the parsing of match branches as for example the following program is rightly rejected
let () =
let rec loop n =
if n = 0 then () else
((); ; loop (n - 1))
in
loop 3
File "t2.ml", line 5, characters 9-10:
Error: Syntax error: ')' expected
File "t2.ml", line 5, characters 4-5:
Error: This '(' might be unmatched
Command exited with code 2.
Compilation unsuccessful after building 1 target (0 cached) in 00:00:00.
Thanks,
Daniel
File attachments
The text was updated successfully, but these errors were encountered: