Removed environment_vars ref #6

Anukriti12 · 2020-07-01T20:48:43Z

No description provided.

Anukriti12

Kindly provide some feedback on this or suggestions on how can I proceed further? I am feeling a bit stuck over here.

lthls · 2020-07-15T09:20:20Z

middle_end/flambda/un_anf.ml

      ignore_function_label label;
-      List.iter (loop ~depth ~environment_vars) args;
-      ignore_debuginfo dbg
+      List.fold_left (loop ~depth ~environment_vars) args;


I assume you're getting a type error here; you'll need to tweak it a bit. (Note: this applies almost exactly to the other List.fold_left occurrences)
loop has type depth:int -> environment_vars:Var.Set.t -> Clambda.ulambda -> Var.Set.t, and List.fold_left has type ('a -> 'b -> 'a) -> 'a -> 'b list -> 'a.
In this context, 'a will be Var.Set.t (the set of environment vars), 'b will be Clambda.ulambda. So the arguments of List.fold_left must be:

First, a function of type Var.Set.t -> Clambda.ulambda -> Var.Set.t. loop ~depth would almost fit this purpose, since it has type environment_vars:Var.Set.t -> Clambda.ulambda -> Var.Set.t, and I think the compiler would allow you to use it (it knows that function types with labeled arguments are compatible with unlabeled ones), but I would suggest writing your own function instead: (fun environment_vars arg -> loop ~depth ~environment_vars arg)

Then, the initial value of type Var.Set.t, in this case the choice of environment_vars should be more or less obvious.

Finally, the list of Clambda.ulambda terms, args in this case.

You also need to recover and return the result from List.fold_left, so you should either switch the ignore and List.fold_left lines, or bind the result of List.fold_left to a variable.

Here's what I would do:

let environment_vars = List.fold_left (fun environment_vars arg -> loop ~depth ~environment_vars arg) environment_vars args in ignore_debuginfo dbg; environment_vars

lthls · 2020-07-15T09:21:27Z

middle_end/flambda/un_anf.ml

-        let environment_vars = V.Set.empty in
+        let environment_vars =
          (match closure_environment_var clos with
           | None -> ()


Here you need to replace () (the unit return value) by environment_vars.

lthls · 2020-07-15T09:31:35Z

middle_end/flambda/un_anf.ml

    | Uclosure (functions, captured_variables) ->
-      List.iter (loop ~depth ~environment_vars) captured_variables;
+      List.fold_left (loop ~depth ~environment_vars) captured_variables;
      List.iter (fun (


This List.iter also needs to be replaced by a List.fold_left call. This one is actually easier than most other cases, since the function argument to List.iter was already an explicit function definition. You just need to add the "accumulator" argument that fold_left expects (environment_vars, in this case) between the fun keyword and the existing argument (the whole ({ Clambda. label; arity; params; return; body; dbg; env; } as clos) is a single argument, this syntax allows to give names to both the argument, clos, and all of its fields).
You will also need to return the new environment_vars at the end of this function (the end of this function is at the right parenthesis that matches the left parenthesis just before the fun keyword).
And like all other occurrences of fold_left, you will need to give a name to the result so that you can return it later.

lthls · 2020-07-15T09:34:03Z

middle_end/flambda/un_anf.ml

          ignore_value_kind return;
          loop ~depth:(depth + 1) ~environment_vars:environment_vars body;
          ignore_debuginfo dbg;
          ignore_var_option env)


The parenthesis on this line ends the function starting on line 144. As discussed above, you'll need to return the new environment_vars, so changing this to ignore_var_option env; environment_vars) should work (although as a style guideline, I strongly recommend adding a newline after each semicolon).

lthls · 2020-07-15T09:36:16Z

middle_end/flambda/un_anf.ml

+      environment_vars      
    | Ulet (_let_kind, _value_kind, var, def, body) ->
      add_definition t (VP.var var) depth;
      loop ~depth ~environment_vars def;


The compiler should complain about ignoring the result of this computation (and if it does not, I will :)).
This should look like let environment_vars = loop ~depth ~environment_vars def in.

lthls · 2020-07-15T09:41:21Z

middle_end/flambda/un_anf.ml

          loop ~depth ~environment_vars branch)
        branches;
-      Option.iter (loop ~depth ~environment_vars) default
+      Option.iter (loop ~depth ~environment_vars) default;


This Option.iter should be replaced with Option.fold and adapted. You should try to see if you can do the adaptation, by looking up the type of Option.fold (you can find it in stdlib/option.mli) and remembering to work done for theList cases.

lthls · 2020-07-15T09:46:39Z

I've left feedback as inline comments. That should help you progress a bit, and we can go over the remaining issues during tomorrow's meeting.

middle_end/flambda/un_anf.ml

lthls · 2020-07-16T07:35:38Z

middle_end/flambda/un_anf.ml

+          ignore_var_option env;
+          environment_vars)
+        in
+        functions;


There's a bit of confusion here: the last List.fold_left, above, is missing some of its arguments, and functions; doesn't make much sense on its own as it's not a computations.

middle_end/flambda/un_anf.ml

lthls · 2020-07-16T07:38:08Z

middle_end/flambda/un_anf.ml

      environment_vars
+      defs
+    in
+    loop ~depth ~environment_vars body;


You forgot the let

lthls · 2020-07-16T07:39:32Z

middle_end/flambda/un_anf.ml

-      loop ~depth ~environment_vars cond;
+      let environment_vars = loop ~depth ~environment_vars cond in
      ignore_int_array us_index_consts;
      Array.iter (loop ~depth ~environment_vars) us_actions_consts;


The Array module also exports a fold_left function, so you can do the same transformation her as for lists

(And two lines below, too)

lthls · 2020-07-16T07:40:39Z

middle_end/flambda/un_anf.ml

      loop ~depth ~environment_vars cond;
-      List.fold_left (fun (str, branch) ->
+      let environment_vars =
+        List.fold_left (fun (str, branch) ->


Missing environment_vars parameter

lthls · 2020-07-16T07:44:05Z

middle_end/flambda/un_anf.ml

-      loop ~depth ~environment_vars body;
+      let environment_vars = loop ~depth ~environment_vars body in
      ignore_var_with_provenance var;
      loop ~depth ~environment_vars handler;


Missing let

lthls · 2020-07-16T07:44:30Z

middle_end/flambda/un_anf.ml

      environment_vars
    | Ufor (var, low, high, direction_flag, body) ->
      ignore_var_with_provenance var;
      loop ~depth ~environment_vars low;


Missing let

lthls · 2020-07-16T07:44:45Z

middle_end/flambda/un_anf.ml

-      loop ~depth ~environment_vars high;
+      let environment_vars = loop ~depth ~environment_vars high in
      ignore_direction_flag direction_flag;
      loop ~depth:(depth + 1) ~environment_vars body;


Missing let

lthls · 2020-07-16T07:45:13Z

middle_end/flambda/un_anf.ml

      environment_vars
    | Usend (meth_kind, e1, e2, args, dbg) ->
      ignore_meth_kind meth_kind;
      loop ~depth ~environment_vars e1;


Missing let

lthls · 2020-07-16T07:45:26Z

middle_end/flambda/un_anf.ml

    | Usend (meth_kind, e1, e2, args, dbg) ->
      ignore_meth_kind meth_kind;
      loop ~depth ~environment_vars e1;
      loop ~depth ~environment_vars e2;


Missing let

middle_end/flambda/un_anf.ml

lthls · 2020-07-28T07:57:35Z

middle_end/flambda/un_anf.ml

      | More_than_one -> (linear, V.Set.add var used, assigned)
      | Assigned -> (linear, V.Set.add var used, V.Set.add var assigned))
      t (V.Set.empty, V.Set.empty, V.Set.empty)
+      acc


I don't think this addition does anything useful. As far as I can tell from a quick glance, acc isn't even defined in this scope.

…l#13294) The toplevel printer detects cycles by keeping a hashtable of values that it has already traversed. However, some OCaml runtime types (at least bigarrays) may be partially uninitialized, and hashing them at arbitrary program points may read uninitialized memory. In particular, the OCaml testsuite fails when running with a memory-sanitizer enabled, as bigarray printing results in reads to uninitialized memory: ``` ==133712==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x4e6d11 in caml_ba_hash /var/home/edwin/git/ocaml/runtime/bigarray.c:486:45 #1 0x52474a in caml_hash /var/home/edwin/git/ocaml/runtime/hash.c:251:35 #2 0x599ebf in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1065:14 #3 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 #4 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 #5 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) #6 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) #7 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37) Uninitialized value was created by a heap allocation #0 0x47d306 in malloc (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x47d306) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37) #1 0x4e7960 in caml_ba_alloc /var/home/edwin/git/ocaml/runtime/bigarray.c:246:12 #2 0x4e801f in caml_ba_create /var/home/edwin/git/ocaml/runtime/bigarray.c:673:10 #3 0x59b8fc in caml_interprete /var/home/edwin/git/ocaml/runtime/interp.c:1058:14 #4 0x5a909a in caml_main /var/home/edwin/git/ocaml/runtime/startup_byt.c:575:9 #5 0x540ccb in main /var/home/edwin/git/ocaml/runtime/main.c:37:3 #6 0x7f0910abb087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) #7 0x7f0910abb14a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 8f53abaad945a669f2bdcd25f471d80e077568ef) #8 0x441804 in _start (/var/home/edwin/git/ocaml/runtime/ocamlrun+0x441804) (BuildId: 7a60eef57e1c2baf770bc38d10d6c227e60ead37) SUMMARY: MemorySanitizer: use-of-uninitialized-value /var/home/edwin/git/ocaml/runtime/bigarray.c:486:45 in caml_ba_hash ``` The only use of hashing in genprintval is to avoid cycles, that is, it is only useful for OCaml values that contain other OCaml values (including possibly themselves). Bigarrays cannot introduce cycles, and they are always printed as "<abstr>" anyway. The present commit proposes to be more conservative in which values are hashed by the cycle detector to avoid this issue: we skip hashing any value with tag above No_scan_tag -- which may not contain any OCaml values. Suggested-by: Gabriel Scherer <[email protected]> Signed-off-by: Edwin Török <[email protected]> Co-authored-by: Edwin Török <[email protected]>

Anukriti12 added 8 commits June 22, 2020 21:44

Removed last commit

accfbb5

made try_depth a labeled argument

642d854

Merge remote-tracking branch 'upstream/trunk' into changes_un_anf

887c989

removed environment_vars reference

3cc5867

few changes - incomplete

71815f0

minor changes

8bf90d4

assigned env-var value

170f093

returned env_vars

4e7ba30

Anukriti12 commented Jul 15, 2020

View reviewed changes

lthls reviewed Jul 15, 2020

View reviewed changes

Anukriti12 added 2 commits July 16, 2020 11:23

list.fold_left usage

861e0ad

option.fold used

f79678e

lthls reviewed Jul 16, 2020

View reviewed changes

middle_end/flambda/un_anf.ml Show resolved Hide resolved

lthls reviewed Jul 16, 2020

View reviewed changes