Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006327OCamlOCaml generalpublic2014-02-18 02:362015-11-28 11:50
Reporterdbuenzli 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusacknowledgedResolutionopen 
PlatformOSOS Version
Product Version4.01.0 
Target VersionlaterFixed in Version 
Summary0006327: Puzzling infinite loop with oddly accepted program
DescriptionAFAIR my OCaml syntax this program should be rejected. It is accepted and running it (osx) results in an infinite loop.

> cat t.ml

let () =
  let rec loop n = match n with
  | 0 -> ()
  | n -> (); ; loop (n - 1)
  in
  loop 3
  
> ocamlbuild t.native
Finished, 4 targets (0 cached) in 00:00:00.
> time ./t.native
^C

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

> cat t2.ml

let () =
  let rec loop n =
    if n = 0 then () else
    ((); ; loop (n - 1))
  in
  loop 3
  
> ocamlbuild t2.native
+ /Users/dbuenzli/.opam/4.01.0/bin/ocamldep.opt -modules t2.ml > t2.ml.depends
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

Tagspatch
Attached Filespatch file icon 0001-forbid-match-as-first-element-of-a-sequence.patch [^] (2,646 bytes) 2014-02-18 14:22 [Show Content]

- Relationships
related to 0004627closed gestion d'un ';' bizarre 
related to 0006961acknowledged Parsing of semicolon followed by binary operator is confusing 

-  Notes
(0010938)
chambart (developer)
2014-02-18 14:26

The problem is that a match case can be terminated with a semicolon, hence this is parsed as:

let () =
  let rec loop n = (match n with
  | 0 -> ()
  | n -> ();) ; loop (n - 1)
  in
  loop 3

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).
(0010939)
dbuenzli (reporter)
2014-02-18 14:50

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
| 0 -> Printf.printf "stop%!";
| n ->
  instr;
  instr;
  (* instr *);
  loop n - 1
 
And suddendly I had a very weird loop that whas printf'ing "stop" but would still continue...

Oh well.
(0010940)
dbuenzli (reporter)
2014-02-18 14:54

Regarding your patch, maybe it should rather be turned into a warning ?
(0010941)
protz (manager)
2014-02-18 17:32

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.

</irony>
(0010957)
doligez (administrator)
2014-02-19 20:44

This is a variant of 0004627, which we decided not to fix. Shall we reverse that decision?
(0010958)
dbuenzli (reporter)
2014-02-19 21:11

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.
(0011863)
doligez (administrator)
2014-07-16 18:12

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).
(0014882)
xleroy (administrator)
2015-11-28 11:50

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 0006961 to "later" than 4.03.

- Issue History
Date Modified Username Field Change
2014-02-18 02:36 dbuenzli New Issue
2014-02-18 14:22 chambart File Added: 0001-forbid-match-as-first-element-of-a-sequence.patch
2014-02-18 14:26 chambart Note Added: 0010938
2014-02-18 14:50 dbuenzli Note Added: 0010939
2014-02-18 14:54 dbuenzli Note Added: 0010940
2014-02-18 17:32 protz Note Added: 0010941
2014-02-19 20:43 doligez Tag Attached: patch
2014-02-19 20:44 doligez Note Added: 0010957
2014-02-19 20:44 doligez Relationship added related to 0004627
2014-02-19 21:11 dbuenzli Note Added: 0010958
2014-05-30 13:46 shinwell Status new => acknowledged
2014-07-16 18:12 doligez Note Added: 0011863
2014-07-16 18:12 doligez Target Version => 4.03.0+dev / +beta1
2015-08-18 17:43 doligez Relationship added related to 0006961
2015-11-28 11:50 xleroy Note Added: 0014882
2015-11-28 11:50 xleroy Target Version 4.03.0+dev / +beta1 => later


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker