Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007789OCamlmiscpublic2018-05-02 20:142018-05-02 22:21
Reportertbrk 
Assigned Togasche 
PriorityhighSeveritymajorReproducibilityalways
StatusresolvedResolutionno change required 
Platformx86_64OSMacOS / Debian LinuxOS Version10.11.6 / 9.4
Product Version4.06.1 
Target VersionFixed in Version4.06.0 
Summary0007789: Mutable field update ignored in closure
DescriptionConsider the following program:

    type data = { mutable n : int; }

    let f { n } t = Printf.printf "n=%d\n%!" n

    let go udata g =
      g 1.0;
      udata.n <- 10;
      g 1.0
    ;;

    let udata = { n = 0 } in
    go udata (f udata);;

In OCaml <= 4.05.0, it prints (correctly):
    n=0
    n=10

In OCaml 4.06.0 and 4.07.0+beta2 (at least), it prints (incorrectly):
    n=0
    n=0
Steps To ReproduceCompile the above program with ocamlc or ocamlopt, or enter it into the top-level.
Additional InformationThe given code is a reduced form from a larger example:
https://github.com/inria-parkas/sundialsml/blob/55a05d3794e98e1d4b3f9953088e41ef6f359bb2/examples/arkode/C_serial/ark_heat1D_adapt.ml#L155 [^]
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0019088)
xleroy (administrator)
2018-05-02 20:26

Ouch, this looks bad.

Are you using the flambda optimizer? To answer this question you can do
ocamlopt -config|grep flambda
(0019089)
tbrk (reporter)
2018-05-02 20:31

Yes, it surprises me that no one else hit it yet. I hope it's not a bĂȘtise on my part.

flambda: false (for both 4.06.0 and 4.07.0+beta2).

I've just started a git bisect.
(0019090)
nojebar (developer)
2018-05-02 20:32

The code for f in 4.06 is:

      (function param/1035
         (funct-body //toplevel//(3)<ghost>:44-80
           (let (n/1012 =o (field 0 param/1035))
             (before //toplevel//(3)<ghost>:50-80
               (function t/1013
                 (funct-body //toplevel//(3)<ghost>:50-80
                   (before //toplevel//(3):54-80
                     (after //toplevel//(3):54-80
                       (apply (field 1 (global Printf!))
                         [0:
                          [11: "n=" [4: 0a 0a 0a [12: '\n' [10: 0a]]]]
                          "n=%d\n%!"]
                         n/1012)))))))))

In 4.05 it is:

      (function param/1238 t/1216
         (funct-body //toplevel//(3)<ghost>:48-84
           (let (n/1215 =a (field 0 param/1238))
             (before //toplevel//(3):58-84
               (after //toplevel//(3):58-84
                 (apply (field 1 (global Printf!))
                   [0:
                    [11: "n=" [4: 0a 0a 0a [12: '\n' [10: 0a]]]] "n=%d\n%!"]
                   n/1215))))))

Somehow f is curried in 4.06 which leads to an early extraction of the value of the udata.n field in 4.06, while it is correctly delayed until f is fully applied in 4.05.
(0019091)
tbrk (reporter)
2018-05-02 20:37

(I don't know why the status changed. I only clicked refresh...)
(0019092)
nojebar (developer)
2018-05-02 20:40

Seems to be related to the compilation of pattern matching as it looks OK if one does not use a pattern to access the field n.
(0019093)
nojebar (developer)
2018-05-02 20:50

I haven't gone through the details, but seems related to https://github.com/ocaml/ocaml/pull/1308 [^]
(0019094)
tbrk (reporter)
2018-05-02 20:59

I'm still chugging through the bisect. Is there are faster way to rebuild than "make clean; ./configure && make world.opt"?
(0019095)
nojebar (developer)
2018-05-02 21:02

No need to waste your time. The commit responsible is https://github.com/ocaml/ocaml/pull/1308. [^] Behaviour being discussed here is an *intended* consequence of that PR.
(0019096)
tbrk (reporter)
2018-05-02 21:05
edited on: 2018-05-02 21:11

Ah, I see. I'm supposed to write:

let f state t = Printf.printf "n=%d\n%!" state.n

I get it now. A bĂȘtise on my part, after all! Thanks for your help. Sorry for the noise.

(0019097)
gasche (developer)
2018-05-02 22:17

This is the second user report that is surprised by the change of behavior. Maybe we should communicate more clearly about this specific change -- I could write a message to the caml-list.
(0019098)
gasche (developer)
2018-05-02 22:21

> Ah, I see. I'm supposed to write:
>
> let f state t = Printf.printf "n=%d\n%!" state.n

Well, I would rather recommend to change

  go udata (f udata);;

into

  go udata (fun t -> f udata t);;

- Issue History
Date Modified Username Field Change
2018-05-02 20:14 tbrk New Issue
2018-05-02 20:26 xleroy Note Added: 0019088
2018-05-02 20:27 xleroy Status new => feedback
2018-05-02 20:31 tbrk Note Added: 0019089
2018-05-02 20:31 tbrk Status feedback => new
2018-05-02 20:32 nojebar Note Added: 0019090
2018-05-02 20:37 tbrk Note Added: 0019091
2018-05-02 20:40 nojebar Note Added: 0019092
2018-05-02 20:50 nojebar Note Added: 0019093
2018-05-02 20:59 tbrk Note Added: 0019094
2018-05-02 21:02 nojebar Note Added: 0019095
2018-05-02 21:05 tbrk Note Added: 0019096
2018-05-02 21:09 tbrk Note Edited: 0019096 View Revisions
2018-05-02 21:11 tbrk Note Edited: 0019096 View Revisions
2018-05-02 22:17 gasche Note Added: 0019097
2018-05-02 22:17 gasche Status new => resolved
2018-05-02 22:17 gasche Fixed in Version => 4.06.0
2018-05-02 22:17 gasche Resolution open => no change required
2018-05-02 22:17 gasche Assigned To => gasche
2018-05-02 22:21 gasche Note Added: 0019098


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker