Merge branch 'bjorn/compiler/map-fixes'

* bjorn/compiler/map-fixes:
  cerl: Remove a clause in fold_map_pairs/3 that will never be reached
  Move grouping of map constructions from v3_core to v3_kernel
  core_pp: Correct printing of map literals
  Strengthen and modernize compile_SUITE
  core_parse: Always fold literal conses
  cerl: Make sure that we preserve the invariants for maps
  cerl_clauses: Fix indentation
  sys_core_fold: Strengthen optimization of letrecs in effect context
  Fix handling of binary map keys in comprehensions
  core_lib: Teach is_var_used/2 to handle keys in map patterns
  warnings_SUITE: Eliminate compiler warning for a shadowed variable
  lc_SUITE: Add shadow/1
  Modernize lc_SUITE

5 files changed, 135 insertions, 57 deletions

diff --git a/lib/compiler/test/compile_SUITE.erl b/lib/compiler/test/compile_SUITE.erl
index 0ff03c4295..1c96abe017 100644
--- a/lib/compiler/test/compile_SUITE.erl
+++ b/lib/compiler/test/compile_SUITE.erl
@@ -748,42 +748,65 @@ env_1(Simple, Target) ->
 %% compile the generated Core Erlang files.
 
 core(Config) when is_list(Config) ->
-    ?line Dog = test_server:timetrap(test_server:minutes(5)),
-    ?line PrivDir = ?config(priv_dir, Config),
-    ?line Outdir = filename:join(PrivDir, "core"),
-    ?line ok = file:make_dir(Outdir),
+    PrivDir = ?config(priv_dir, Config),
+    Outdir = filename:join(PrivDir, "core"),
+    ok = file:make_dir(Outdir),
 
-    ?line Wc = filename:join(filename:dirname(code:which(?MODULE)), "*.beam"),
-    ?line TestBeams = filelib:wildcard(Wc),
-    ?line Abstr = [begin {ok,{Mod,[{abstract_code,
+    Wc = filename:join(filename:dirname(code:which(?MODULE)), "*.beam"),
+    TestBeams = filelib:wildcard(Wc),
+    Abstr = [begin {ok,{Mod,[{abstract_code,
 				    {raw_abstract_v1,Abstr}}]}} = 
 			     beam_lib:chunks(Beam, [abstract_code]),
 			 {Mod,Abstr} end || Beam <- TestBeams],
-    ?line Res = test_lib:p_run(fun(F) -> do_core(F, Outdir) end, Abstr),
-    ?line test_server:timetrap_cancel(Dog),
-    Res.
-
+    test_lib:p_run(fun(F) -> do_core(F, Outdir) end, Abstr).
     
 do_core({M,A}, Outdir) ->
     try
-	{ok,M,Core} = compile:forms(A, [to_core,report]),
-	CoreFile = filename:join(Outdir, atom_to_list(M)++".core"),
-	CorePP = core_pp:format(Core),
-	ok = file:write_file(CoreFile, CorePP),
-	case compile:file(CoreFile, [clint,from_core,binary]) of
-	    {ok,M,_} ->
-		ok = file:delete(CoreFile);
-	    Other ->
-		io:format("*** core_lint failure '~p' for ~s\n",
-			  [Other,CoreFile]),
-		error
-	end
-    catch Class:Error ->
+	do_core_1(M, A, Outdir)
+    catch
+	throw:{error,Error} ->
+	    io:format("*** compilation failure '~p' for module ~s\n",
+		      [Error,M]),
+	    error;
+	Class:Error ->
 	    io:format("~p: ~p ~p\n~p\n",
 		      [M,Class,Error,erlang:get_stacktrace()]),
 	    error
     end.
 
+do_core_1(M, A, Outdir) ->
+    {ok,M,Core0} = compile:forms(A, [to_core]),
+    CoreFile = filename:join(Outdir, atom_to_list(M)++".core"),
+    CorePP = core_pp:format(Core0),
+    ok = file:write_file(CoreFile, CorePP),
+
+    %% Parse the .core file and return the result as Core Erlang Terms.
+    Core = case compile:file(CoreFile, [report_errors,from_core,no_copt,to_core,binary]) of
+	       {ok,M,Core1} -> Core1;
+	       Other -> throw({error,Other})
+	   end,
+    ok = file:delete(CoreFile),
+
+    %% Compile as usual (including optimizations).
+    compile_forms(Core, [clint,from_core,binary]),
+
+    %% Don't optimize to test that we are not dependent
+    %% on the Core Erlang optmimization passes.
+    %% (Example of a previous bug: The core_parse pass
+    %% would not turn map literals into #c_literal{}
+    %% records; if sys_core_fold was run it would fix
+    %% that; if sys_core_fold was not run v3_kernel would
+    %% crash.)
+    compile_forms(Core, [clint,from_core,no_copt,binary]),
+
+    ok.
+
+compile_forms(Forms, Opts) ->
+    case compile:forms(Forms, [report_errors|Opts]) of
+	{ok,[],_} ->  ok;
+	Other -> throw({error,Other})
+    end.
+
 %% Compile to Beam assembly language (.S) and then try to
 %% run .S through the compiler again.
 
diff --git a/lib/compiler/test/lc_SUITE.erl b/lib/compiler/test/lc_SUITE.erl
index 398398a397..6c5b34498b 100644
--- a/lib/compiler/test/lc_SUITE.erl
+++ b/lib/compiler/test/lc_SUITE.erl
@@ -18,12 +18,12 @@
 %%
 -module(lc_SUITE).
 
--author('[email protected]').
 -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, 
 	 init_per_group/2,end_per_group/2,
 	 init_per_testcase/2,end_per_testcase/2,
 	 basic/1,deeply_nested/1,no_generator/1,
-	 empty_generator/1,no_export/1]).
+	 empty_generator/1,no_export/1,shadow/1,
+	 effect/1]).
 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -31,10 +31,18 @@ suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
     test_lib:recompile(?MODULE),
-    [basic, deeply_nested, no_generator, empty_generator, no_export].
+    [{group,p}].
 
 groups() -> 
-    [].
+    [{p,test_lib:parallel(),
+      [basic,
+       deeply_nested,
+       no_generator,
+       empty_generator,
+       no_export,
+       shadow,
+       effect
+      ]}].
 
 init_per_suite(Config) ->
     Config.
@@ -59,34 +67,34 @@ end_per_testcase(Case, Config) when is_atom(Case), is_list(Config) ->
     ok.
 
 basic(Config) when is_list(Config) ->
-    ?line L0 = lists:seq(1, 10),
-    ?line L1 = my_map(fun(X) -> {x,X} end, L0),
-    ?line L1 = [{x,X} || X <- L0],
-    ?line L0 = my_map(fun({x,X}) -> X end, L1),
-    ?line [1,2,3,4,5] = [X || X <- L0, X < 6],
-    ?line [4,5,6] = [X || X <- L0, X > 3, X < 7],
-    ?line [] = [X || X <- L0, X > 32, X < 7],
-    ?line [1,3,5,7,9] = [X || X <- L0, odd(X)],
-    ?line [2,4,6,8,10] = [X || X <- L0, not odd(X)],
-    ?line [1,3,5,9] = [X || X <- L0, odd(X), X =/= 7],
-    ?line [2,4,8,10] = [X || X <- L0, not odd(X), X =/= 6],
+    L0 = lists:seq(1, 10),
+    L1 = my_map(fun(X) -> {x,X} end, L0),
+    L1 = [{x,X} || X <- L0],
+    L0 = my_map(fun({x,X}) -> X end, L1),
+    [1,2,3,4,5] = [X || X <- L0, X < 6],
+    [4,5,6] = [X || X <- L0, X > 3, X < 7],
+    [] = [X || X <- L0, X > 32, X < 7],
+    [1,3,5,7,9] = [X || X <- L0, odd(X)],
+    [2,4,6,8,10] = [X || X <- L0, not odd(X)],
+    [1,3,5,9] = [X || X <- L0, odd(X), X =/= 7],
+    [2,4,8,10] = [X || X <- L0, not odd(X), X =/= 6],
 
     %% Append is specially handled.
-    ?line [1,3,5,9,2,4,8,10] = [X || X <- L0, odd(X), X =/= 7] ++
+    [1,3,5,9,2,4,8,10] = [X || X <- L0, odd(X), X =/= 7] ++
 	[X || X <- L0, not odd(X), X =/= 6],
 
     %% Guards BIFs are evaluated in guard context. Weird, but true.
-    ?line [{a,b,true},{x,y,true,true}] = [X || X <- tuple_list(), element(3, X)],
+    [{a,b,true},{x,y,true,true}] = [X || X <- tuple_list(), element(3, X)],
 
     %% Filter expressions with andalso/orelse.
-    ?line "abc123" = alphanum("?abc123.;"),
+    "abc123" = alphanum("?abc123.;"),
 
     %% Error cases.
-    ?line [] = [{xx,X} || X <- L0, element(2, X) == no_no_no],
-    ?line {'EXIT',_} = (catch [X || X <- L1, list_to_atom(X) == dum]),
-    ?line [] = [X || X <- L1, X+1 < 2],
-    ?line {'EXIT',_} = (catch [X || X <- L1, odd(X)]),
-    ?line fc([x], catch [E || E <- id(x)]),
+    [] = [{xx,X} || X <- L0, element(2, X) == no_no_no],
+    {'EXIT',_} = (catch [X || X <- L1, list_to_atom(X) == dum]),
+    [] = [X || X <- L1, X+1 < 2],
+    {'EXIT',_} = (catch [X || X <- L1, odd(X)]),
+    fc([x], catch [E || E <- id(x)]),
     ok.
 
 tuple_list() ->
@@ -116,12 +124,12 @@ deeply_nested_1() ->
 	X16 <- [4],X17 <- [3],X18 <- [fun() -> X16+X17 end],X19 <- [2],X20 <- [1]].
 
 no_generator(Config) when is_list(Config) ->
-    ?line Seq = lists:seq(-10, 17),
-    ?line [no_gen_verify(no_gen(A, B), A, B) || A <- Seq, B <- Seq],
+    Seq = lists:seq(-10, 17),
+    [no_gen_verify(no_gen(A, B), A, B) || A <- Seq, B <- Seq],
 
     %% Literal expression, for coverage.
-    ?line [a] = [a || true],
-    ?line [a,b,c] = [a || true] ++ [b,c],
+    [a] = [a || true],
+    [a,b,c] = [a || true] ++ [b,c],
     ok.
 
 no_gen(A, B) ->    
@@ -174,13 +182,40 @@ no_gen_eval(Fun, Res) ->
 no_gen_one_more(A, B) -> A + 1 =:= B.
 
 empty_generator(Config) when is_list(Config) ->
-    ?line [] = [X || {X} <- [], (false or (X/0 > 3))],
+    [] = [X || {X} <- [], (false or (X/0 > 3))],
     ok.
 
 no_export(Config) when is_list(Config) ->
     [] = [ _X = a || false ] ++ [ _X = a || false ],
     ok.
 
+%% Test that variables in list comprehensions are
+%% correctly shadowed.
+
+shadow(Config) when is_list(Config) ->
+    Shadowed = nomatch,
+    _ = id(Shadowed),				%Eliminate warning.
+    L = [{Shadowed,Shadowed+1} || Shadowed <- lists:seq(7, 9)],
+    [{7,8},{8,9},{9,10}] = id(L),
+    [8,9] = id([Shadowed || {_,Shadowed} <- id(L),
+			    Shadowed < 10]),
+    ok.
+
+effect(Config) when is_list(Config) ->
+    [{42,{a,b,c}}] =
+	do_effect(fun(F, L) ->
+			  [F({V1,V2}) ||
+			      #{<<1:500>>:=V1,<<2:301>>:=V2} <- L],
+			  ok
+		  end, id([#{},x,#{<<1:500>>=>42,<<2:301>>=>{a,b,c}}])),
+    ok.
+
+do_effect(Lc, L) ->
+    put(?MODULE, []),
+    F = fun(V) -> put(?MODULE, [V|get(?MODULE)]) end,
+    ok = Lc(F, L),
+    lists:reverse(erase(?MODULE)).
+
 id(I) -> I.
     
 fc(Args, {'EXIT',{function_clause,[{?MODULE,_,Args,_}|_]}}) -> ok;
diff --git a/lib/compiler/test/map_SUITE.erl b/lib/compiler/test/map_SUITE.erl
index dbac61765b..bc5ae803c6 100644
--- a/lib/compiler/test/map_SUITE.erl
+++ b/lib/compiler/test/map_SUITE.erl
@@ -319,6 +319,12 @@ t_update_exact(Config) when is_list(Config) ->
     {'EXIT',{badarg,_}} = (catch M0#{42=>v1,42.0:=v2,42:=v3}),
     {'EXIT',{badarg,_}} = (catch <<>>#{nonexisting:=val}),
     {'EXIT',{badarg,_}} = (catch M0#{<<0:257>> := val}), %% limitation
+
+    %% A workaround for a bug allowed an empty map to be updated.
+    {'EXIT',{badarg,_}} = (catch (id(#{}))#{a:=1}),
+    {'EXIT',{badarg,_}} = (catch #{}#{a:=1}),
+    Empty = #{},
+    {'EXIT',{badarg,_}} = (catch Empty#{a:=1}),
     ok.
 
 t_update_values(Config) when is_list(Config) ->
diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl
index 033988e4c8..74efed7588 100644
--- a/lib/compiler/test/match_SUITE.erl
+++ b/lib/compiler/test/match_SUITE.erl
@@ -22,7 +22,8 @@
 	 init_per_group/2,end_per_group/2,
 	 pmatch/1,mixed/1,aliases/1,match_in_call/1,
 	 untuplify/1,shortcut_boolean/1,letify_guard/1,
-	 selectify/1,underscore/1,match_map/1,coverage/1]).
+	 selectify/1,underscore/1,match_map/1,map_vars_used/1,
+      coverage/1]).
 	 
 -include_lib("test_server/include/test_server.hrl").
 
@@ -36,7 +37,7 @@ groups() ->
     [{p,[parallel],
       [pmatch,mixed,aliases,match_in_call,untuplify,
        shortcut_boolean,letify_guard,selectify,
-       underscore,match_map,coverage]}].
+       underscore,match_map,map_vars_used,coverage]}].
 
 
 init_per_suite(Config) ->
@@ -412,6 +413,19 @@ do_match_map(#s{map=#{key:=Val}}=S) ->
     %% Would crash with a 'badarg' exception.
     S#s{t=Val}.
 
+map_vars_used(Config) when is_list(Config) ->
+    {some,value} = do_map_vars_used(a, b, #{{a,b}=>42,v=>{some,value}}),
+    ok.
+
+do_map_vars_used(X, Y, Map) ->
+    case {X,Y} of
+	T ->
+	    %% core_lib:is_var_used/2 would not consider T used.
+	    #{T:=42,v:=Val} = Map,
+	    Val
+    end.
+
+
 coverage(Config) when is_list(Config) ->
     %% Cover beam_dead.
     ok = coverage_1(x, a),
diff --git a/lib/compiler/test/warnings_SUITE.erl b/lib/compiler/test/warnings_SUITE.erl
index be0348a92d..6e5a7d35e9 100644
--- a/lib/compiler/test/warnings_SUITE.erl
+++ b/lib/compiler/test/warnings_SUITE.erl
@@ -699,10 +699,10 @@ run(Config, Tests) ->
 %% Compiles a test module and returns the list of errors and warnings.
 
 run_test(Conf, Test0, Warnings) ->
-    Mod = "warnings_"++test_lib:uniq(),
-    Filename = Mod ++ ".erl",
+    Module = "warnings_"++test_lib:uniq(),
+    Filename = Module ++ ".erl",
     ?line DataDir = ?privdir,
-    Test = ["-module(", Mod, "). ", Test0],
+    Test = ["-module(", Module, "). ", Test0],
     ?line File = filename:join(DataDir, Filename),
     ?line Opts = [binary,export_all,return|Warnings],
     ?line ok = file:write_file(File, Test),