Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options for silencing warnings for behaviours #9020

Merged

Conversation

bjorng
Copy link
Contributor

@bjorng bjorng commented Nov 5, 2024

This commit adds the following compiler options for suppressing warnings having to do with behaviours:

  • nowarn_conflicting_behaviours
  • nowarn_undefined_behaviour_func
  • nowarn_undefined_behaviour
  • nowarn_undefined_behaviour_callbacks
  • nowarn_ill_defined_behaviour_callbacks
  • nowarn_ill_defined_optional_callbacks

Closes #8985

@bjorng bjorng added team:VM Assigned to OTP team VM feature testing currently being tested, tag is used by OTP internal CI labels Nov 5, 2024
@bjorng bjorng self-assigned this Nov 5, 2024
Copy link
Contributor

github-actions bot commented Nov 5, 2024

CT Test Results

    3 files    417 suites   1h 12m 31s ⏱️
2 867 tests 2 820 ✅ 47 💤 0 ❌
7 982 runs  7 932 ✅ 50 💤 0 ❌

Results for commit b38327e.

♻️ 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

// Erlang/OTP Github Action Bot

This commit adds the following compiler options for suppressing
warnings having to do with behaviours:

* nowarn_conflicting_behaviours
* nowarn_undefined_behaviour_func
* nowarn_undefined_behaviour
* nowarn_undefined_behaviour_callbacks
* nowarn_ill_defined_behaviour_callbacks
* nowarn_ill_defined_optional_callbacks

Closes erlang#8985
@bjorng bjorng force-pushed the bjorn/stdlib/behaviour-warnings/GH-8985/OTP-19334 branch from d0a1d6e to 0f24dee Compare November 6, 2024 04:20
@bjorng bjorng requested a review from garazdawi November 7, 2024 09:46
@michalmuskala
Copy link
Contributor

Should there be an option for completely skipping any behaviour-related checks?

In most cases those checks can be performed by xref or dialyzer without actually loading the module into the VM. Offloading the check to those tools (in projects that use them) could simplify compilation significantly and increase scalability of the compiler by removing compile-time dependencies between modules.

@bjorng
Copy link
Contributor Author

bjorng commented Nov 8, 2024

Should there be an option for completely skipping any behaviour-related checks?

Yes, that makes sense. I've added nowarn_behaviours to disable all behaviour-related warnings.

Copy link
Contributor

@kikofernandez kikofernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good PR. I just added some suggestions in case they help

Comment on lines +1293 to +1301
add_behaviour_warning(Anno, Warning, St) when is_tuple(Warning) ->
Tag = element(1, Warning),
case is_warn_enabled(Tag, St) of
true ->
add_warning(Anno, Warning, St);
false ->
St
end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_behaviour_warning(Anno, Warning, St) when is_tuple(Warning) ->
Tag = element(1, Warning),
case is_warn_enabled(Tag, St) of
true ->
add_warning(Anno, Warning, St);
false ->
St
end.
add_behaviour_warning(Anno, Warning, St) when is_tuple(Warning) ->
Tag = element(1, Warning),
Command = fun () -> add_warning(Anno, Warning, St) end),
if_warn_enabled(Tag, St, Command.
if_warn_enabled(Tag, St, Command) ->
case is_warn_enabled(Tag, St) of
true ->
Command();
false ->
St
end.

This is a suggestion to a common pattern in this PR.
Take it if you find it useful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I will save it for a clean-up of erl_lint that I am planning to do.

Comment on lines +1263 to +1269
check_behaviour(St) ->
case is_warn_enabled(behaviours, St) of
true ->
behaviour_check(St#lint.behaviour, St);
false ->
St
end.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
check_behaviour(St) ->
case is_warn_enabled(behaviours, St) of
true ->
behaviour_check(St#lint.behaviour, St);
false ->
St
end.
check_behaviour(St) ->
Command = fun () -> behaviour_check(St#lint.behaviour, St) end,
if_warn_enabled(behaviours, St, Command).

Take this is you like, must be applied with the other suggestion below

@bjorng bjorng merged commit b7fa407 into erlang:master Nov 11, 2024
21 checks passed
@bjorng bjorng deleted the bjorn/stdlib/behaviour-warnings/GH-8985/OTP-19334 branch November 11, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An option to silence behaviour-related warnings
3 participants