Make options in the -compile attribute take precedence

[bjorng]

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.

Fixes #6979

[github-actions]

CT Test Results

    2 files    324 suites   8m 59s ⏱️
  804 tests   802 ✅ 2 💤 0 ❌
5 350 runs  5 348 ✅ 2 💤 0 ❌

Results for commit 06d0e7c.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[michalmuskala]

Are we sure all the passes in the compiler use the "last option wins" approach, so that the documentation, which option takes precedence is correct? It's definitely true of erl_lint, but I haven't looked at other passes

[bjorng]

Are we sure all the passes in the compiler use the "last option wins" approach, so that the documentation, which option takes precedence is correct?

No, other passes generally take the first option. However, the other options generally don't come in pairs (one to enable and one to disable a feature), so the order should not matter. I will take another look to see whether there any other option where the order would matter.

[michalmuskala]

If that's the case, I wonder if it would make sense to instead change the implementation in erl_lint to take the first option into account, rather than last - it should actually be more efficient too.

[ilya-klyuchnikov]

Unfortunately, this change doesn't work for the particular combination

• command line option: +warn_missing_spec
• module option: -compile(nowarn_missing_spec).

(So, it doesn't fix #6979)

The PR changes the phase compile_directives and this phase is executed AFTER lint_module (erl_lint) phase - see

otp/lib/compiler/src/compile.erl

Lines 1584 to 1630 in f11b248

	standard_passes() ->
	[?pass(transform_module),
	{iff,makedep_side_effect,?pass(makedep_and_output)},
	{iff,makedep,[
	?pass(makedep),
	{unless,binary,?pass(makedep_output)}
	]},
	{iff,makedep,done},
	{iff,'dpp',{listing,"pp"}},
	?pass(lint_module),
	{unless,no_docs,?pass(beam_docs)},
	?pass(remove_doc_attributes),
	{iff,'P',{src_listing,"P"}},
	{iff,'to_pp',{done,"P"}},
	{iff,'dabstr',{listing,"abstr"}}
	| abstr_passes(verified_abstr)].
	abstr_passes(AbstrStatus) ->
	case AbstrStatus of
	non_verified_abstr -> [{unless, no_lint, ?pass(lint_module)},
	{unless,no_docs,?pass(beam_docs)},
	?pass(remove_doc_attributes)];
	verified_abstr -> []
	end ++
	[
	%% Add all -compile() directives to #compile.options
	?pass(compile_directives),
	{delay,[{iff,debug_info,?pass(save_abstract_code)}]},
	{iff,line_coverage,{pass,sys_coverage}},
	?pass(expand_records),
	{iff,'dexp',{listing,"expand"}},
	{iff,'E',?pass(legalize_vars)},
	{iff,'E',{src_listing,"E"}},
	{iff,'to_exp',{done,"E"}},
	%% Conversion to Core Erlang.
	?pass(core),
	{iff,'dcore',{listing,"core"}},
	{iff,'to_core0',{done,"core"}}
	| core_passes(verified_core)].

Additionally, erl_lint itself reads options from the -compile attribute and puts them in front (of current options):

otp/lib/stdlib/src/erl_lint.erl

Lines 689 to 698 in f11b248

	module(Forms, FileName, Opts0) ->
	%% FIXME Hmm, this is not coherent with the semantics of features
	%% We want the options given on the command line to take
	%% precedence over options in the module.
	Opts = compiler_options(Forms) ++ Opts0,
	St = forms(Forms, start(FileName, Opts)),
	return_status(St).
	compiler_options(Forms) ->
	lists:flatten([C || {attribute,_,compile,C} <- Forms]).

And lastly, - whether the check is enabled is configured through bool_option :

otp/lib/stdlib/src/erl_lint.erl

Lines 750 to 752 in f11b248

	{missing_spec,
	bool_option(warn_missing_spec, nowarn_missing_spec,
	false, Opts)},

And since bool_option is encoded using foldl - the last warn_missing_spec wins anyway:

otp/lib/stdlib/src/erl_lint.erl

Lines 102 to 106 in f11b248

	bool_option(On, Off, Default, Opts) ->
	foldl(fun (Opt, _Def) when Opt =:= On -> true;
	(Opt, _Def) when Opt =:= Off -> false;
	(_Opt, Def) -> Def
	end, Default, Opts).

[bjorng]

@ilya-klyuchnikov Thanks! I have reverted this PR for now. The plan is to implement it correctly (with test cases) in a new PR and to include it in RC2.