diff options
author | Björn Gustavsson <[email protected]> | 2015-01-14 07:05:21 +0100 |
---|---|---|
committer | Björn Gustavsson <[email protected]> | 2015-01-14 14:44:22 +0100 |
commit | 85701edb6bdfe7c616a8486542dc2ca4bd787113 (patch) | |
tree | 15c6a1a0e1612c1dd51852dd2a15b68e942f1460 /lib/compiler/test/match_SUITE.erl | |
parent | a4449c481a6c887166097fd28f545876f6791625 (diff) | |
download | otp-85701edb6bdfe7c616a8486542dc2ca4bd787113.tar.gz otp-85701edb6bdfe7c616a8486542dc2ca4bd787113.tar.bz2 otp-85701edb6bdfe7c616a8486542dc2ca4bd787113.zip |
sys_core_fold: Correct optimization of 'case'
The optimization of a 'case' statement could lead to incorrect
code that would cause an exception at run-time.
Here is an example to show how the optimization went wrong. Start
with the following code:
f({r,#{key:=Val},X}=S) ->
case S of
{r,_,_} ->
setelement(3, Val, S)
end.
(The record operations have already been translated to the
corresponding tuple operations.) The first step in case_opt/3 is
to substitute S to obtain:
f({r,#{key:=Val},X}=S) ->
case {r,#{key:=Val},X} of
{r,_,_} ->
setelement(3, Val, S)
end.
After that substitution the 'case' can be simplified to:
f({r,#{key:=Val},_}=S) ->
case #{key:=Val} of
NewVar ->
setelement(3, Val, S)
end.
That is the result from case_opt/3. Now eval_case/2 notices
that since there is only one clause left in the 'case', the
'case' can eliminated:
f({r,#{key:=Val},_}=S) ->
NewVar = #{key:=Val},
setelement(3, Val, S).
Since the map construction may have a side effect, it was not
eliminated, but assigned to a variable that is never used.
The problem is that '#{key:=Val}' is fine as a pattern, but in a
construction of a new map, the '=>' operator must be used. So the
map construction will fail, generating an exception.
As a conservative correction for a maintenance release, we will
abort the 'case' optimization if the substitution into the 'case'
expression is anything but data items (tuples, conses, or
literals) or variables.
Reported-by: Dmitry Aleksandrov
Diffstat (limited to 'lib/compiler/test/match_SUITE.erl')
-rw-r--r-- | lib/compiler/test/match_SUITE.erl | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl index ae7d764535..e5aaf49d6f 100644 --- a/lib/compiler/test/match_SUITE.erl +++ b/lib/compiler/test/match_SUITE.erl @@ -22,7 +22,7 @@ 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,coverage/1]). + selectify/1,underscore/1,match_map/1,coverage/1]). -include_lib("test_server/include/test_server.hrl"). @@ -35,7 +35,8 @@ all() -> groups() -> [{p,test_lib:parallel(), [pmatch,mixed,aliases,match_in_call,untuplify, - shortcut_boolean,letify_guard,selectify,underscore,coverage]}]. + shortcut_boolean,letify_guard,selectify, + underscore,match_map,coverage]}]. init_per_suite(Config) -> @@ -400,6 +401,17 @@ underscore(Config) when is_list(Config) -> _ = is_list(Config), ok. +-record(s, {map,t}). + +match_map(Config) when is_list(Config) -> + Map = #{key=>{x,y},ignore=>anything}, + #s{map=Map,t={x,y}} = do_match_map(#s{map=Map}), + ok. + +do_match_map(#s{map=#{key:=Val}}=S) -> + %% Would crash with a 'badarg' exception. + S#s{t=Val}. + coverage(Config) when is_list(Config) -> %% Cover beam_dead. ok = coverage_1(x, a), |