Skip to content

Commit

Permalink
Make options in the -compile attribute take precedence
Browse files Browse the repository at this point in the history
Change the compiler option processing order so that options given
in the `compile()` attribute takes precedence over options given
to the compiler, which in turn takes precedence over options given
in the environment.

This order makes most sense, as each module might need customized
options.

While at it, remove the undocumented `strict_record_updates` /
`no_strict_record_updates` options. Their naming no longer make any
sense, because record updates are always strict (that is, the source
record must have the correct tag and size). Incorporate the
behavior of `strict_record_updates` to update the record by matching
and building a new tuple into the `dialyzer` option. When dialyzer
is not used, records are updated using `setelement/3`, which is more
efficient in the JIT.

(This is the second attempt to fix erlang#6979, as erlang#8093 did not really
work.)
  • Loading branch information
bjorng committed Mar 4, 2024
1 parent ab7b354 commit 513d482
Show file tree
Hide file tree
Showing 5 changed files with 297 additions and 35 deletions.
19 changes: 18 additions & 1 deletion lib/compiler/src/compile.erl
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,23 @@ compiler recursively from inside a parse transform.
The list can be retrieved with `env_compiler_options/0`.
## Order of Compiler Options
Options given in the `compile()` attribute in the source code takes
precedence over options given to the compiler, which in turn takes
precedence over options given in the environment.
A later compiler option takes precedence over an earlier one in the
option list. Example:
```
compile:file(something, [nowarn_missing_spec,warn_missing_spec]).
```
Warnings will be emitted for functions without specifications, unless
the source code for module `something` contains a `compile(nowarn_missing_spec)`
attribute.
## Inlining
The compiler can do function inlining within an Erlang
Expand Down Expand Up @@ -792,7 +809,7 @@ Module:format_error(ErrorDescriptor)
CompRet :: comp_ret().

file(File, Opts) when is_list(Opts) ->
do_compile({file,File}, Opts++env_default_opts());
do_compile({file,File}, env_default_opts() ++ Opts);
file(File, Opt) ->
file(File, [Opt|?DEFAULT_OPTIONS]).

Expand Down
252 changes: 247 additions & 5 deletions lib/compiler/test/compile_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
bc_options/1, deterministic_include/1, deterministic_paths/1,
compile_attribute/1, message_printing/1, other_options/1,
transforms/1, erl_compile_api/1, types_pp/1, bs_init_writable/1,
annotations_pp/1
annotations_pp/1, option_order/1
]).

suite() -> [{ct_hooks,[ts_install_cth]}].
Expand All @@ -59,7 +59,8 @@ all() ->
env_compiler_options, custom_debug_info, bc_options,
custom_compile_info, deterministic_include, deterministic_paths,
compile_attribute, message_printing, other_options, transforms,
erl_compile_api, types_pp, bs_init_writable, annotations_pp].
erl_compile_api, types_pp, bs_init_writable, annotations_pp,
option_order].

groups() ->
[].
Expand Down Expand Up @@ -906,10 +907,10 @@ strict_record(Config) when is_list(Config) ->
{ok,M} = c:c(M, [no_strict_record_tests|Opts]),
Turtle = test_sloppy(),

%% The option first given wins.
{ok,M} = c:c(M, [no_strict_record_tests,strict_record_tests|Opts]),
Turtle = test_sloppy(),
%% The option last given wins.
{ok,M} = c:c(M, [strict_record_tests,no_strict_record_tests|Opts]),
Turtle = test_sloppy(),
{ok,M} = c:c(M, [no_strict_record_tests,strict_record_tests|Opts]),
Turtle = test_strict(),

%% Default (possibly influenced by ERL_COMPILER_OPTIONS).
Expand Down Expand Up @@ -2120,6 +2121,146 @@ get_annotations(Key, [_|Lines]) ->
get_annotations(_, []) ->
[].

option_order(Config) ->
Ts = [{spec1,
~"""
-compile(nowarn_missing_spec).
foo() -> ok.
""",
[], %Environment
[warn_missing_spec],
[]},
{spec2,
~"""
foo() -> ok.
""",
[{"ERL_COMPILER_OPTIONS", "warn_missing_spec"}],
[nowarn_missing_spec],
[]},
{spec3,
~"""
-compile(nowarn_missing_spec).
foo() -> ok.
""",
[{"ERL_COMPILER_OPTIONS", "nowarn_missing_spec"}],
[warn_missing_spec],
[]},
{spec4,
~"""
-compile(warn_missing_spec).
foo() -> ok.
""",
[{"ERL_COMPILER_OPTIONS", "nowarn_missing_spec"}],
[],
{warnings,[{{2,1},erl_lint,{missing_spec,{foo,0}}}]}
},
{spec5,
~"""
-compile([warn_missing_spec,nowarn_missing_spec]).
foo() -> ok.
""",
[{"ERL_COMPILER_OPTIONS", "nowarn_missing_spec"}],
[warn_missing_spec],
[]},
{records1,
~"""
-record(r, {x,y}).
rec_test(#r{x=X,y=Y}) -> X + Y.
""",
[],
[strict_record_tests],
fun(M) ->
try M:rec_test({r,1,2,3}) of
3 ->
fail()
catch
error:function_clause ->
ok
end
end},
{records2,
~"""
-record(r, {x,y}).
rec_test(R) -> R#r.x + R#r.y.
""",
[],
[no_strict_record_tests],
fun(M) ->
3 = M:rec_test({r,1,2,3}),
ok
end},
{records3,
~"""
-compile(no_strict_record_tests).
-record(r, {x,y}).
rec_test(R) -> R#r.x + R#r.y.
""",
[],
[strict_record_tests],
fun(M) ->
3 = M:rec_test({r,1,2,3}),
ok
end},
{records4,
~"""
-record(r, {x,y}).
rec_test(#r{x=X,y=Y}) -> X + Y.
""",
[{"ERL_COMPILER_OPTIONS", "strict_record_tests"}],
[],
fun(M) ->
try M:rec_test({r,1,2,3}) of
3 ->
fail()
catch
error:function_clause ->
ok
end
end},
{records5,
~"""
-record(r, {x,y}).
rec_test(R) -> R#r.x + R#r.y.
""",
[{"ERL_COMPILER_OPTIONS", "strict_record_tests"}],
[no_strict_record_tests],
fun(M) ->
3 = M:rec_test({r,1,2,3}),
ok
end},
{records6,
~"""
-compile(no_strict_record_tests).
-record(r, {x,y}).
rec_test(R) -> R#r.x + R#r.y.
""",
[{"ERL_COMPILER_OPTIONS", "strict_record_tests"}],
[],
fun(M) ->
3 = M:rec_test({r,1,2,3}),
ok
end},
{records7,
~"""
-record(r, {x,y}).
rec_test(R) -> R#r.x + R#r.y.
""",
[{"ERL_COMPILER_OPTIONS", "no_strict_record_tests"}],
[no_strict_record_tests, strict_record_tests],
fun(M) ->
try M:rec_test({r,1,2,3}) of
3 ->
fail()
catch
error:{badrecord,{r,1,2,3}} ->
ok
end
end}

],
run(Config, Ts),
ok.

%%%
%%% Utilities.
%%%
Expand Down Expand Up @@ -2149,3 +2290,104 @@ is_lfe_module(File, Ext) ->
"lfe_" ++ _ -> true;
_ -> false
end.

%% Compiles a test module and returns the list of errors and warnings.

run(Config, Tests) ->
F = fun({N,P,Env,Ws,Run}, _BadL) when is_function(Run, 1) ->
case catch run_test(Config, P, Env, Ws, Run) of
ok ->
ok;
Bad ->
io:format("~nTest ~p failed. Expected~n ~p~n"
"but got~n ~p~n", [N, ok, Bad]),
fail()
end;
({N,P,Env,Ws,Expected}, BadL)
when is_list(Expected); is_tuple(Expected) ->
io:format("### ~s\n", [N]),
case catch run_test(Config, P, Env, Ws, none) of
Expected ->
BadL;
Bad ->
io:format("~nTest ~p failed. Expected~n ~p~n"
"but got~n ~p~n", [N, Expected, Bad]),
fail()
end
end,
lists:foldl(F, [], Tests).

run_test(Conf, Test0, Env, Options, Run) ->
run_test_putenv(Env),
Module = "warnings" ++ test_lib:uniq(),
Filename = Module ++ ".erl",
DataDir = proplists:get_value(priv_dir, Conf),
Test1 = ["-module(", Module, "). -file( \"", Filename, "\", 1). ", Test0],
Test = iolist_to_binary(Test1),
File = filename:join(DataDir, Filename),
Opts = [binary,export_all,return|Options],
ok = file:write_file(File, Test),

%% Compile once just to print all warnings (and cover more code).
_ = compile:file(File, [binary,export_all,report|Options]),

%% Test result of compilation.
{ok, Mod, Beam, Warnings} = compile:file(File, Opts),
_ = file:delete(File),

if
is_function(Run, 1) ->
{module,Mod} = code:load_binary(Mod, "", Beam),
ok = Run(Mod),
run_test_unsetenv(Env),
true = code:delete(Mod),
_ = code:purge(Mod),
ok;
Run =:= none ->
run_test_unsetenv(Env),
Res = get_warnings(Warnings),
case Res of
[] ->
[];
{warnings, Ws} ->
print_warnings(Ws, Test),
Res
end
end.

run_test_putenv(Env) ->
_ = [_ = os:putenv(Name, Value) || {Name,Value} <- Env],
ok.

run_test_unsetenv(Env) ->
_ = [_ = os:unsetenv(Name) || {Name,_Value} <- Env],
ok.

get_warnings([]) ->
[];
get_warnings(WsL) ->
case WsL of
[{_File,Ws}] -> {warnings, Ws};
_ -> {warnings, WsL}
end.

print_warnings(Warnings, Source) ->
Lines = binary:split(Source, <<"\n">>, [global]),
Cs = [print_warning(W, Lines) || W <- Warnings],
io:put_chars(Cs),
ok.

print_warning({{LineNum,Column},Mod,Data}, Lines) ->
Line0 = lists:nth(LineNum, Lines),
<<Line1:(Column-1)/binary,_/binary>> = Line0,
Spaces = re:replace(Line1, <<"[^\t]">>, <<" ">>, [global]),
CaretLine = [Spaces,"^"],
[io_lib:format("~p:~p: ~ts\n",
[LineNum,Column,Mod:format_error(Data)]),
Line0, "\n",
CaretLine, "\n\n"];
print_warning(_, _) ->
[].

fail() ->
ct:fail(failed).
2 changes: 1 addition & 1 deletion lib/dialyzer/src/dialyzer_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ sets_filter([Mod|Mods], ExpTypes) ->

src_compiler_opts() ->
[no_copt, to_core, binary, return_errors,
no_inline, strict_record_tests, strict_record_updates,
no_inline, strict_record_tests,
dialyzer, no_spawn_compiler_process].

-spec format_errors([{module(), string()}]) -> [string()].
Expand Down
Loading

0 comments on commit 513d482

Please sign in to comment.