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

Handle simple guard chains #525

Merged
merged 3 commits into from
Apr 4, 2023
Merged

Conversation

bartekgorny
Copy link
Contributor

@bartekgorny bartekgorny commented Mar 31, 2023

...so that a second guard uses type inferred by the first one

@bartekgorny
Copy link
Contributor Author

I found out that it still doesn't work as expected, will push an update soon.

@bartekgorny
Copy link
Contributor Author

done

test/should_pass/guard.erl Outdated Show resolved Hide resolved
test/should_pass/guard.erl Outdated Show resolved Hide resolved
@erszcz
Copy link
Collaborator

erszcz commented Apr 4, 2023

This look good ✅

It doesn't make the mechanism complete, as there are more cases of how guards can be combined, but it's a step in the right direction, so I think it's good to merge as is.

For the context, we can have the following combinations of guards:

-module(guards).

single(A) when is_integer(A) ->
    ok.

comma(A) when is_integer(A), A > 5 ->
    ok.

semicolon(A) when is_integer(A); is_float(A) ->
    ok.

comma_semicolon(A) when is_integer(A), A > 5; is_float(A) ->
    ok.

andalso_(A) when is_integer(A) andalso A > 5 ->
    ok.

orelse_(A) when is_integer(A) orelse is_float(A) ->
    ok.

Which result in the following AST forms:

20> rp(beam_lib:chunks(guards, [debug_info])).
{ok,{guards,
        [{debug_info,
             {debug_info_v1,erl_abstract_code,
                 {[{attribute,{1,1},file,{"guards.erl",1}},
                   {attribute,{1,2},module,guards},
                   {function,
                       {3,1},
                       single,1,
                       [{clause,
                            {3,1},
                            [{var,{3,8},'A'}],
                            [[{call,
                                  {3,16},
                                  {atom,{3,16},is_integer},
                                  [{var,{3,27},'A'}]}]],
                            [{atom,{4,5},ok}]}]},
                   {function,
                       {6,1},
                       comma,1,
                       [{clause,
                            {6,1},
                            [{var,{6,7},'A'}],
                            [[{call,{6,15},{atom,{6,15},is_integer},[{var,{6,26},'A'}]},
                              {op,{6,32},'>',{var,{6,30},'A'},{integer,{6,34},5}}]],
                            [{atom,{7,5},ok}]}]},
                   {function,
                       {9,1},
                       semicolon,1,
                       [{clause,
                            {9,1},
                            [{var,{9,11},'A'}],
                            [[{call,{9,19},{atom,{9,19},is_integer},[{var,{9,30},'A'}]}],
                             [{call,{9,34},{atom,{9,34},is_float},[{var,{9,43},'A'}]}]],
                            [{atom,{10,5},ok}]}]},
                   {function,
                       {12,1},
                       comma_semicolon,1,
                       [{clause,
                            {12,1},
                            [{var,{12,17},'A'}],
                            [[{call,
                                  {12,25},
                                  {atom,{12,25},is_integer},
                                  [{var,{12,36},'A'}]},
                              {op,{12,42},'>',{var,{12,40},'A'},{integer,{12,44},5}}],
                             [{call,
                                  {12,47},
                                  {atom,{12,47},is_float},
                                  [{var,{12,56},'A'}]}]],
                            [{atom,{13,5},ok}]}]},
                   {function,
                       {15,1},
                       andalso_,1,
                       [{clause,
                            {15,1},
                            [{var,{15,10},'A'}],
                            [[{op,{15,32},
                                  'andalso',
                                  {call,{15,18},{atom,{15,18},is_integer},[{var,{15,29},'A'}]},
                                  {op,{15,42},'>',{var,{15,40},'A'},{integer,{15,44},5}}}]],
                            [{atom,{16,5},ok}]}]},
                   {function,
                       {18,1},
                       orelse_,1,
                       [{clause,
                            {18,1},
                            [{var,{18,9},'A'}],
                            [[{op,{18,31},
                                  'orelse',
                                  {call,{18,17},{atom,{18,17},is_integer},[{var,{18,28},'A'}]},
                                  {call,
                                      {18,38},
                                      {atom,{18,38},is_float},
                                      [{var,{18,47},'A'}]}}]],
                            [{atom,{19,5},ok}]}]},
                   {eof,{20,1}}],
                  [debug_info]}}}]}}
ok

With this PR we'll be handling not only single but also comma style guards. We also match the structure of non-nested andalso or orelse guards as well as comparison operators, but they're later filtered out by the check_guard_call check.

@erszcz erszcz changed the title fold guard chain Handle simple guard chains Apr 4, 2023
@erszcz erszcz merged commit 1f32b8c into josefs:master Apr 4, 2023
@erszcz erszcz requested a review from zuiderkwast April 4, 2023 11:08
@erszcz
Copy link
Collaborator

erszcz commented Apr 4, 2023

@zuiderkwast I've merged it already, but if you have any notes please let us know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants