Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006327OCamlOCaml generalpublic2014-02-18 02:362014-02-19 21:11
Reporterdbuenzli 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusnewResolutionopen 
PlatformOSOS Version
Product Version4.01.0 
Target VersionFixed 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 

-  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.

- 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


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker