Merge branch 'feature/finish-for-collect-loop-impl' into 'dev'

fix last problems with nested for collection loop

See merge request ligolang/ligo!197

src/passes/2-simplify/pascaligo.ml

@@ -12,6 +12,7 @@ let pseq_to_list = function
   | None -> []
   | Some lst -> npseq_to_list lst
 let get_value : 'a Raw.reg -> 'a = fun x -> x.value
+let is_compiler_generated = fun name -> String.contains name '#'
 
 module Errors = struct
   let unsupported_cst_constr p =
@@ -116,17 +117,6 @@ module Errors = struct
     ] in
     error ~data title message
 
-  let unsupported_deep_access_for_collection for_col =
-    let title () = "deep access in loop over collection" in
-    let message () =
-      Format.asprintf "currently, we do not support deep \
-                       accesses in loops over collection" in
-    let data = [
-      ("pattern_loc",
-       fun () -> Format.asprintf "%a" Location.pp_lift @@ for_col.Region.region)
-    ] in
-    error ~data title message
-
   (* Logging *)
 
   let simplifying_instruction t =
@@ -1001,6 +991,16 @@ and simpl_for_int : Raw.for_int -> (_ -> expression result) result = fun fi ->
 
     2) Detect the free variables and build a list of their names
        (myint and myst in the previous example)
+       Free variables are simply variables being assigned.
+       Note:  In the case of a nested loops, assignements to a compiler
+              generated value (#COMPILER#acc) correspond to variables
+              that were already renamed in the inner loop.
+              e.g :
+              ```
+              #COMPILER#acc.myint := #COMPILER#acc.myint + #COMPILER#elt ;
+              #COMPILER#acc.myst  := #COMPILER#acc.myst ^ "to" ;
+              ```
+              They must not be considered as free variables
 
     3) Build the initial record (later passed as 2nd argument of
       `MAP/SET/LIST_FOLD`) capturing the environment using the
@@ -1014,6 +1014,10 @@ and simpl_for_int : Raw.for_int -> (_ -> expression result) result = fun fi ->
             - references to the iterated value ==> variable `#COMPILER#elt_value`
             in the case of a set/list:
             - references to the iterated value ==> variable `#COMPILER#elt`
+       Note: In the case of an inner loop capturing variable from an outer loop
+             the free variable name can be `#COMPILER#acc.Y` and because we do not
+             capture the accumulator record in the inner loop, we don't want to 
+             generate `#COMPILER#acc.#COMPILER#acc.Y` but `#COMPILER#acc.Y`
 
     5) Append the return value to the body
 
@@ -1047,10 +1051,8 @@ and simpl_for_collect : Raw.for_collect -> (_ -> expression result) result = fun
     (fun (prev : type_name list) (ass_exp : expression) ->
       match ass_exp.expression with
       | E_assign ( name , _ , _ ) ->
-        if (String.contains name '#') then
+        if is_compiler_generated name then ok prev
-          ok prev
+        else ok (name::prev)
-        else
-          ok (name::prev)
       | _ -> ok prev )
     []
     for_body in
@@ -1061,17 +1063,18 @@ and simpl_for_collect : Raw.for_collect -> (_ -> expression result) result = fun
   (* STEP 4 *)
   let replace exp =
     match exp.expression with
-    (* replace references to fold accumulator as rhs *)
+    (* replace references to fold accumulator as lhs *)
     | E_assign ( name , path , expr ) -> (
-      match path with
+      let path' = List.filter
-      | [] -> ok @@ e_assign "#COMPILER#acc" [Access_record name] expr
+        ( fun el ->
-      (* This fails for deep accesses, see LIGO-131 LIGO-134 *)
+          match el with
-      | _ ->
+          | Access_record name -> not @@ is_compiler_generated name
-        (* ok @@ e_assign "#COMPILER#acc" ((Access_record name)::path) expr) *)
+          | _ -> true )
-        fail @@ unsupported_deep_access_for_collection fc.block )
+        ((Access_record name)::path) in
+      ok @@ e_assign "#COMPILER#acc" path' expr)
     | E_variable name -> (
       if (List.mem name captured_name_list) then
-        (* replace references to fold accumulator as lhs *)
+        (* replace references to fold accumulator as rhs *)
         ok @@ e_accessor (e_variable "#COMPILER#acc") [Access_record name]
       else match fc.collection with
       (* loop on map *)
@@ -1107,16 +1110,10 @@ and simpl_for_collect : Raw.for_collect -> (_ -> expression result) result = fun
     let ( arg_access: Types.access_path -> expression ) = e_accessor (e_variable "arguments") in
     ( match fc.collection with
       | Map _ ->
-        (* let acc          = arg_access [Access_tuple 0 ; Access_tuple 0] in
+        let acc          = arg_access [Access_tuple 0 ] in
         let collec_elt_v = arg_access [Access_tuple 1 ; Access_tuple 0] in
-        let collec_elt_k = arg_access [Access_tuple 1 ; Access_tuple 1] in *)
+        let collec_elt_k = arg_access [Access_tuple 1 ; Access_tuple 1] in
-        (* The above should work, but not yet (see LIGO-131) *)
-        let temp_kv      = arg_access [Access_tuple 1] in
-        let acc          = arg_access [Access_tuple 0] in
-        let collec_elt_v = e_accessor (e_variable "#COMPILER#temp_kv") [Access_tuple 0] in
-        let collec_elt_k = e_accessor (e_variable "#COMPILER#temp_kv") [Access_tuple 1] in
         e_let_in ("#COMPILER#acc", None) acc @@
-        e_let_in ("#COMPILER#temp_kv", None) temp_kv @@
         e_let_in ("#COMPILER#collec_elt_k", None) collec_elt_v @@
         e_let_in ("#COMPILER#collec_elt_v", None) collec_elt_k (for_body)
       | _ ->

src/test/contracts/loop.ligo

@@ -134,20 +134,26 @@ function for_collection_map_k (var nee : unit) : string is block {
     end
 } with st
 
-// function nested_for_collection (var nee : unit) : (int*string) is block {
+function nested_for_collection (var nee : unit) : (int*string) is block {
-//   var myint : int := 0;
+  var myint : int := 0;
-//   var myst : string := "";
+  var mystoo : string := "";
-//   var mylist : list(int) := list 1 ; 2 ; 3 end ;
+  var mylist : list(int) := list 1 ; 2 ; 3 end ;
-//   for i : int in list mylist
+  var mymap : map(string,string) := map " one" -> "," ; "two" -> " " end ;
-//   begin
+
-//     myint := myint + i ;
+  for i in list mylist
-//     var myset : set(string) := set "1" ; "2" ; "3" end ;
+  begin
-//     for st : string in set myset
+    myint := myint + i ;
-//     begin
+    var myset : set(string) := set "1" ; "2" ; "3" end ;
-//       myst := myst ^ st ;
+    for st in set myset
-//     end
+    begin
-//   end
+      mystoo := mystoo ^ st ;
-// } with (myint,myst)
+      for k -> v in map mymap
+      begin
+        mystoo := mystoo ^ k ^ v ;
+      end
+    end
+  end
+} with (myint,mystoo)
 
 function dummy (const n : nat) : nat is block {
   while False block { skip }

src/test/integration_tests.ml

@@ -812,9 +812,10 @@ let loop () : unit result =
   let%bind () =
     let expected = (e_int 20) in
     expect_eq program "for_collection_comp_with_acc" input expected in
-  (* let%bind () =
+  let%bind () =
-    let expected = e_pair (e_int 6) (e_string "123123123") in
+    let expected = e_pair (e_int 6)
-    expect_eq program "nested_for_collection" input expected in *)
+      (e_string "1 one,two 2 one,two 3 one,two 1 one,two 2 one,two 3 one,two 1 one,two 2 one,two 3 one,two ") in
+    expect_eq program "nested_for_collection" input expected in
   let%bind () =
     let ez lst =
       let open Ast_simplified.Combinators in