Age | Commit message (Collapse) | Author |
|
Moving a fun into a guard may cause code that is not accepted
by beam_validator.
|
|
Add more filename/line number annotations while translating to
Core Erlang in v3_core, and ensure that sys_core_fold retains
existing annotations. The goal is to avoid that sys_core_fold
generate warnings with "no_file" instead of a filename.
|
|
Rewrite code such as:
X = not_a_fun,
X()
to:
error({badfun,not_a_fun})
Also generate a warning.
|
|
|
|
Binary matching can be confusing. For example:
1> <<-1>> = <<-1>>.
** exception error: no match of right hand side value <<"ÿ">>
2>
When constructing binaries, the value will be masked to fit in
the binary segment. But no such masking happens when matching
binaries.
One solution that we considered was to do the same masking when
matching. We have rejected that solution for several reasons:
* Masking in construction is highly controversial and by some
people considered a bad design decision.
* While masking of unsigned numbers can be understood, masking of
signed numbers it not easy to understand.
* Then there is the question of backward compatibility. Adding
masking to matching would mean that clauses that did not match
earlier would start to match. That means that code that has
never been tested will be executed. Code that has not been
tested will usually not work.
Therefore, we have decided to warn for binary patterns that cannot
possibly match.
While we are it, we will also warn for the following example where
size for a binary segment is invalid:
bad_size(Bin) ->
BadSize = bad_size,
<<42:BadSize>> = Bin.
That example would crash the HiPE compiler because the BEAM compiler
would generate a bs_get_integer2 instruction with an invalid size
field. We can avoid that crash if sys_core_fold not only warns for bad
binary pattern, but also removes the clauses that will not match.
Reported-by: http://bugs.erlang.org/browse/ERL-44
Reported-by: Kostis Sagonas
|
|
As a preparation for checking binary patterns we will add
var_list/2 that will work as pattern_list/2 but is guaranteed
not to throw an exception. That way, we will only have to use
try...catch for the few remaining calls to pattern_list/2.
|
|
Save work the *extremely* common case that the guard is
a literal.
|
|
* maint:
Fix missing filename and line number in warning
Conflicts:
lib/compiler/test/bs_match_SUITE.erl
|
|
When the 'bin_opt_info' is given, warnings without filenames
and line numbers could sometimes be produced:
no_file: Warning: INFO: matching non-variables after
a previous clause matching a variable will prevent delayed
sub binary optimization
The reason for the missing information is that #c_alias{} records lack
location information. There are several ways to fix the problem. The
easiest seems to be to get the location information from the
code).
Noticed-by: José Valim
|
|
Knowing that a BIF returns an integer makes it possible to
replace '==' with the cheaper '=:=' test.
|
|
|
|
* bjorn/compiler/spurious-warning:
sys_core_fold: Eliminate warnings for unused terms in effect context
sys_core_fold: Eliminate warnings for unused terms
|
|
The optimization introduced in 0a0d39d351fc could cause spurious
warnings of the type: "a term is constructed, but never used".
That would happen for constructs in effect context.
To avoid those warnings, we will need to apply warning suppression
also in effect context.
|
|
Using Maps as type information container speedups files like cow_http_hd.erl
by ~500ms. Previously spent ~60% of the time in orddict:store/3.
|
|
|
|
|
|
The optimization introduced in 0a0d39d351fc would cause spurious
warnings of the type: "a term is constructed, but never used".
To avoid the warning, we must mark not only tuples and lists as
compiler_generated, but also each element. We must also propagate
compiler_generated annotations in lets. For example, if we have:
let <X -| ['compiler_generated']> = 42 in X + 1
we must propagate the compiler_generated annotation to the literal
when do constant propagation:
42 -| ['compiler_generated'] + 1
|
|
86fbd6d76d strengthened type optimization in lets. As a result of
the stronger optimizations, special care had to be taken to
suppress false warnings.
It turns out that false warnings can still slip through. Slapping
on a 'compiler_generated' annotation at the top-level of a
complex term such as #c_tuple{} may not suppress all warnings.
We will need to go deeper into the term to eliminate all warnings.
|
|
According to EEP-43 for maps, a 'badmap' exception should be
generated when an attempt is made to update non-map term such as:
<<>>#{a=>42}
That was not implemented in the OTP 17.
José Valim suggested that we should take the opportunity to
improve the errors coming from map operations:
http://erlang.org/pipermail/erlang-questions/2015-February/083588.html
This commit implement better errors from map operations similar
to his suggestion.
When a map update operation (Map#{...}) or a BIF that expects a map
is given a non-map term, the exception will be:
{badmap,Term}
This kind of exception is similar to the {badfun,Term} exception
from operations that expect a fun.
When a map operation requires a key that is not present in a map,
the following exception will be raised:
{badkey,Key}
José Valim suggested that the exception should be
{badkey,Key,Map}. We decided not to do that because the map
could potentially be huge and cause problems if the error
propagated through links to other processes.
For BIFs, it could be argued that the exceptions could be simply
'badmap' and 'badkey', because the bad map and bad key can be found in
the argument list for the BIF in the stack backtrace. However, for the
map update operation (Map#{...}), the bad map or bad key will not be
included in the stack backtrace, so that information must be included
in the exception reason itself. For consistency, the BIFs should raise
the same exceptions as update operation.
If more than one key is missing, it is undefined which of
keys that will be reported in the {badkey,Key} exception.
|
|
For a long time, there has been an optimization for:
{V1,V2,...} = case Expr of Pat -> ... {Val1,Val2,...}; ... end
that avoids building the tuples. The construct looks like this
in Core Erlang:
let <V> = case X of
Pattern -> {Y,Z}
end
in case V of
{A,B} -> A+B
end
The current optimization will try to replace the second 'case'
with a 'let':
let <A,B> = case X of
Pattern -> <Y,Z>
end
in A+B
Simple variations of the construct would prevent the optimizations;
for example this one:
let <V> = case X of
Pattern -> {'ok',Val}
end
in case V of
{ok,Val} -> Val
end
The problem is that the optimization tries to do too much. By
making the optimization do less and have it depend on other
optimizations to finish the job, it will become more powerful.
Thus we can rewrite the code like this:
let <V1,V2> = case X of
Pattern -> <'ok',Val>
end
in let <V> = {V1,V2}
in case V of
{ok,Val} -> Val
end
Note that the second case is unchanged. The other optimizations in the
sys_core_fold module will optimize the second 'case' and eliminate the
building of the tuple.
|
|
Optimize away 'not' in sys_core_fold instead of in beam_block
and beam_dead, as we can do a better job in sys_core_fold.
I modified the test suite temporarily to never turn off Core Erlang
modifications and looked at the coverage. With the new optimizations
active in sys_core_fold, the code in beam_block and beam_dead did not
find a single 'not' that it could optimize. That proves that the new
optimization is at least as good as the old one. Manually, I could
also verify that the new optimization would optimize some variations
of 'not' that the old one would not handle.
|
|
More aggressive optimizations that we plan to introduce could cause
spurious compiler warnings.
|
|
The 'try' ... 'catch' is problematic. Firstly, if no optimization
is possible, an exception will always be thrown. Secondly, bugs
in the code will go unnoticed.
|
|
'=:=' is a cheaper operation than '==', so we should always
use '=:=' if the result will be the same as if '==' were used.
|
|
|
|
Make sure that we take extract all possible type information when
optimizing a 'let' construct.
Since the stronger optimization may generate false warnings, we also
need to take special care to suppress false warnings.
|
|
If we have a sequence of put_map_* instructions operating on the
same map, it will be more efficient if we can have one is_map/2
instruction before put_map_* instructions, so that each put_map_*
does not need to test whether the argument is a map.
|
|
649d6e73 simplified opt_simple_let_2/6 a little bit too much,
so that some list comprehensions in effect context were not
properly tail-recursive.
|
|
In cd1eaf0116190, opt_simple_let_2/6 was updated to do the same
optimizations in 'value' and 'effect' context.
Coalesce the clauses for 'value' and 'effect' context to one to
make it clear that they do the same thing.
|
|
The code for inlining high-order functions from the lists module
is quite annoying when you try to navigate the sys_core_fold
module. Break out the code into its own module.
|
|
Those functions allow us to clean up some more code.
|
|
Introduce access functions to hide the low-level details of how
type information is implemented.
|
|
Essentially, core_lib:literal_value/1 became useless when literals
were introduced in R12. Since we always create #c_literal{} records
whenever possible, literal_value/1 would *only* succeed when it was
passed a #c_literal{} argument.
|
|
We are about to deprecate core_lib:get_anno/1 and core_lib:set_anno/2,
so we should stop using them in the compiler.
|
|
Rename add_scope/2 to sub_add_scope/2 to be similar in naming as
the other functions that operates on #sub{} (in particular,
sub_subst_scope/1). Also, move the definition to be near to the
other sub_* functions.
|
|
* bjorn/compiler/dup-bug-fix/OTP-12453:
Teach case_opt/3 to avoid unnecessary building
sys_core_fold: Optimize let statements more aggressively
Suppress warnings for expressions that are assigned to '_'
trace_bif_SUITE: Ensure that a call to time/0 is not removed
|
|
* maint:
Update primary bootstrap
Correct unsafe optimization of '==' and '/='
Conflicts:
bootstrap/lib/compiler/ebin/sys_core_fold.beam
|
|
Since '=:=' is cheaper than '==', the compiler tries to replace
'==' with '=:=' if the result of comparison will be the same.
As an example:
V == {a,b}
can be rewritten to:
V =:= {a,b}
since the literal on the right side contains no numeric values
that '==' would compare differently to '=:='.
With the introduction of maps, we will need to take them into
account. Since the comparison of maps is planned to change in 18.0,
we will be very conservative and only do the optimization if
both keys and values are non-numeric.
|
|
Given this code:
f(S) ->
F0 = F1 = {S,S},
[F0,F1].
case_opt/3 would "optimize" it like this:
f(S) ->
F1 = {S,S},
F0 = {S,S},
[F0,F1].
Similarly, this code:
g({a,_,_}=T) ->
{b,
[_,_] = [T,none],
x}.
would be rewritten to:
g({a,Tmp1,Tmp2}=T) ->
Tmp3 = {a,Tmp1,Tmp2},
{b,
[Tmp3,none],
x}.
where the tuple is rebuilt instead of using the T variable.
Rewrite case_opt/3 to be more careful while optimizing.
|
|
I originally decided that in 'value' context, rewriting a let statement
where the variables were not in the body to a sequence was not worth
it, because the variables would be unused in only one let in a
thousand lets (roughly).
I have reconsidered.
The main reason is that if we do the rewrite, core_lib:is_var_used/2
will be used much more frequently, which will help us to find bugs
in it sooner.
Another reason is that the way letify/2 is currently implemented
with its own calls to core_lib:is_var_used/2 is only safe as long
as all the bindings are independent of each other. We could make
letify/2 smarter, but if we introduce this new optimization there
is no need.
Measuring compilation speed, I have not seen any significant slowdown.
It seems that although core_lib:is_var_used/2 is called much more
frequently, most calls will be fast because is_var_used/2 will quickly
find a use of the variable.
Also add a test case to cover a line opt_guard_try/1 that was
no longer covered.
|
|
In c34ad2d5, the compiler learned to silence some warnings for
expressions that were explicitly assigned to the '_' variable,
as in this example:
_ = list_to_integer(S),
ok
That commit intentionally only made it possible to silence warnings
for BIFs that could cause an exception. Warnings would still be
produced for:
_ = date(),
ok
because date/0 can never fail and thus making the call completely
useless. The reasoning was that such warnings can always be
eliminated by eliminating the offending code.
While that is true, there is the question about rules and their
consistency. It is surprising that '_' can be used to silence
some warnings, but has no effect on other warnings.
Therefore, we will teach the compiler to silence warnings for
the following constructs:
* Calls to safe BIFs such as date/0
* Expressions that will cause an exception such as 'X/0'
* Terms that are built but not used, such as '{x,X}'
|
|
* maint:
Update primary bootstrap
Be more careful about map patterns when evalutating element/2
Do not convert map patterns to map expressions
Conflicts:
bootstrap/lib/compiler/ebin/sys_core_fold.beam
lib/compiler/test/match_SUITE.erl
|
|
We must not convert map patterns to map expressions.
|
|
In code such as:
case {a,Map} of
{a,#{}}=T ->
T
end
we must NOT rewrite a map pattern to a map expression like this:
case Map of
#{} ->
{a,#{}}
end
because the pattern '#{}' will match any map, but the expression
'#{}' will construct an empty map.
|
|
* 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
|
|
sys_core_fold:eval_element/3 attempts to evaluate calls to element/2
at compile time or to warn when the call will obviously fail. For
example:
element(1, [a])
will obviously fail and eval_element/3 will produce a warning.
eval_element/3 uses the helper functions is_not_integer/1 and
is_not_tuple/1 to test whether the arguments are known to be
incorrect. The clauses that attempt to match #c_map{} in those
helper function will never be executed, because #c_map{} will
never occur directly in an argument for a function call.
For example, code such as:
element(1, #{a=>Val})
will be translated to:
let <NewVar> = #{a=>Val}
in element(1, NewVar)
since maps are not considered safe (some map operations may
cause an exception at run time).
|
|
v3_core is careful to always create literals whenever possible.
Correct core_parse so it, too, always creates literals out
of literal conses. With that correction, we can remove the
workaround in sys_core_fold (introduced in 26a5dea3cb5e101)
that handles non-literal flags in a binary.
|
|
We used to evaluate the body of a 'letrec' in value context, even
if the 'letrec' was being evaluated in effect context. In most
cases, the context does not matter because the body is usually
just an 'apply' which will never be optimized away.
However, in the case of incorrect code described in the previous
commit, it does matter. We can find such bad code by evaluating
the body in effect context. For example, if we have the following
incorrect code:
letrec
f/1 = fun(A) -> ... <use of Var> ...
in let Var = <<2:301>>
in apply(Arg)
If the letrec is evaluated in effect context, the code will be
reduced to:
letrec
f/1 = fun(A) -> ... <use of Var> ...
in seq Var = <<2:301>> do apply(Arg)
Now Var will be unbound and a later compiler pass will crash to
ensure that the bad Core Erlang code is noticed.
Also add a test case to ensure that the compiler crashes if the
bug fixed in the previous commit re-surfaces.
|
|
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
|
|
The scope is supposed to contain all variables that are currently
live. We need this information for certain optimizations to
avoid capturing a name (a name that is in the scope must be renamed;
for an example, see move_let_into_expr/2 or any function that calls
sub_subst_scope/1). We also use the scope to optimize sub_del_var/2
and sub_is_val/2.
When optimizing case expressions, the scope could be reset to an
empty list (because sub_new/0 was called instead of sub_new/1).
That could cause name capture if inlining was turned on.
As simple way to force this bug is to uncomment the
"-define(DEBUG, 1)." near the beginning of the file. Without this
correction, most files in the test suite fail to compile.
|