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

Idea: More warnings related to clauses that can never match (map/keys #8558

Closed
PragTob opened this issue Jun 10, 2024 · 2 comments · Fixed by #8600
Closed

Idea: More warnings related to clauses that can never match (map/keys #8558

PragTob opened this issue Jun 10, 2024 · 2 comments · Fixed by #8600
Assignees
Labels
enhancement team:VM Assigned to OTP team VM

Comments

@PragTob
Copy link
Contributor

PragTob commented Jun 10, 2024

Dear lovely erlang team,

thanks for a lot of your awesome work powering erlang and everything else running on the BEAM! 💚 It's much appreciated.

Is your feature request related to a problem? Please describe.

For full reference I come from this proposal I made to elixir-core where José told me that the warnings right now are coming straight from the erlang compiler.

What's the problem?

We have some warnings that function clauses can not match, most prominently a catch all variable match will warn for all the ones below it:

/home/tobi/github/playground/src/map_pattern.erl:6:1: Warning: this clause for fun_map/1 cannot match because a previous clause at line 4 always matches
%    6| fun_map(#{}) ->
%     | ^

However, there are relatively easy pattern matches we can make, such as for an empty map, that won't warn if we do further pattern matches down the line:

-module(map_pattern).
-export([fun_map/1]).

% this warns
% commented out to show that there are no warnings
% fun_map(_Var) ->
%     io:format("Just a variable!~n");
% these don't warn
fun_map(#{}) ->
    io:format("The map is empty~n");
fun_map(#{first := FirstValue}) ->
    io:format("The map has a 'first' key with value: ~p~n", [FirstValue]);
fun_map(#{first := FirstValue, second := SecondValue}) ->
    io:format("The map has a 'first' & 'second' key with value: ~p~n ~p~n", [
        FirstValue, SecondValue
    ]).

Repo with the file: https://github.com/PragTob/erlang_playground/blob/main/src/map_pattern.erl

The case of matching against an empty map and then more maps below is the easiest as the latter clauses will never match.

The problem can be generalized though, if the keys of a previous map match are a sub set of the ones in a match after it, the latter can never match.

Of course, is a simple example and gets muddier as soon as other arguments with pattern matches and/or guards are involved.

Describe the solution you'd like

I wish the cases described above would also emit a warning. I have seen some of these in production code, with long modules and lots of function heads it can happen. It'd be great if the compiler was so friendly and extended its helpful warnings a bit here.

Describe alternatives you've considered
I think the alternative is "deal with it" or make the checks part of another tool if it should not be in core erlang. I can guess, that it may be more complex than I think or that the overhead during compilation to achieve this is not desirable.

Additional context
Nothing, thank you for all your outstanding work!

@bjorng bjorng added the team:VM Assigned to OTP team VM label Jun 10, 2024
@bjorng bjorng self-assigned this Jun 10, 2024
@bjorng
Copy link
Contributor

bjorng commented Jun 20, 2024

Thanks for your idea and for your thanks! 😄

The linked pull request introduces warnings for some map patterns that will never match. Note that there is no guarantee that there will warnings for all kind of map patterns that cannot possibly match.

@PragTob
Copy link
Contributor Author

PragTob commented Jun 20, 2024

@bjorng Thanks a lot/tack så mycket!

Go have a bunny:

IMG20230429191639

@bjorng bjorng closed this as completed in 8a8ea04 Jun 26, 2024
bjorng added a commit that referenced this issue Jun 26, 2024
…H-8558/OTP-19141

Add opportunistic warnings for shadowed map patterns
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants