Warning not emitted on string matching clauses

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Warning not emitted on string matching clauses

Pierre Fenoll-2
-module(wat_clauses).
-export([authenticate_nouns/1]). 
authenticate_nouns([{<<"user_auth">>, _}]) -> 'true';
authenticate_nouns([{<<"user_auth">>, [<<"recovery">>]}]) -> hi;
authenticate_nouns(_Nouns) -> 'false'.
 
In this code (or above), the second clause will never match (because of source-order).
But erlc does not complain about it.

Ferd on IRC mentioned that the compiler might feel free to reorder clauses.
But the call wat_clauses:authenticate_nouns([{<<"user_auth">>, [<<"recovery">>]}])
returns true instead of hi, so no reordering is done.

Dialyzer does not complain either.

Am I right in thinking we should have a warning about a never matching clause?

-- 
Pierre Fenoll


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Fred Hebert-2
On 04/01, Pierre Fenoll wrote:
>Ferd on IRC mentioned that the compiler might feel free to reorder
>clauses.

The compiler would only reorder clauses that are free to do so.

For example:

    f(a) -> 1;
    f(b) -> 2;
    f(X) when is_atom(X) -> 3;
    f([_|_]) -> 4;
    f({_}) -> 5.

Would let the compiler possibly reorder 1 & 2, and maybe 4 & 5. But the
presence of a guard clause or ambiguous matches would prevent it from
doing more than that, if my understanding is accurate.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Björn-Egil Dahlberg-2
The compiler doesn't really "reorder" the clauses. It builds a match tree depending on types, values and variables. In the current implementation it does so by inspecting all clauses left to right.

You can inspect the matching tree by compiling to kernel, i.e. erlc +to_kernel t.erl

// Björn-Egil

2015-04-01 20:57 GMT+02:00 Fred Hebert <[hidden email]>:
On 04/01, Pierre Fenoll wrote:
Ferd on IRC mentioned that the compiler might feel free to reorder clauses.

The compiler would only reorder clauses that are free to do so.

For example:

   f(a) -> 1;
   f(b) -> 2;
   f(X) when is_atom(X) -> 3;
   f([_|_]) -> 4;
   f({_}) -> 5.

Would let the compiler possibly reorder 1 & 2, and maybe 4 & 5. But the presence of a guard clause or ambiguous matches would prevent it from doing more than that, if my understanding is accurate.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Robert Virding
Also any reordering the compiler does in the way it tests the patterns is guaranteed not to change the semantics of the original. The resultant code will always behave as if the clauses had been tested top-to-bottom as written.

Robert


On 1 April 2015 at 21:28, Björn-Egil Dahlberg <[hidden email]> wrote:
The compiler doesn't really "reorder" the clauses. It builds a match tree depending on types, values and variables. In the current implementation it does so by inspecting all clauses left to right.

You can inspect the matching tree by compiling to kernel, i.e. erlc +to_kernel t.erl

// Björn-Egil

2015-04-01 20:57 GMT+02:00 Fred Hebert <[hidden email]>:
On 04/01, Pierre Fenoll wrote:
Ferd on IRC mentioned that the compiler might feel free to reorder clauses.

The compiler would only reorder clauses that are free to do so.

For example:

   f(a) -> 1;
   f(b) -> 2;
   f(X) when is_atom(X) -> 3;
   f([_|_]) -> 4;
   f({_}) -> 5.

Would let the compiler possibly reorder 1 & 2, and maybe 4 & 5. But the presence of a guard clause or ambiguous matches would prevent it from doing more than that, if my understanding is accurate.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions



_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Erik Søe Sørensen
In reply to this post by Pierre Fenoll-2

It would of course make sense to have the compiler warn in this case.
However - if my understanding of the compiler implementation is correct - matching of binaries is handled specially, and apparently in such a manner that it doesn't realise that the two binary patterns are identical.
/Erik

Den 01/04/2015 20.43 skrev "Pierre Fenoll" <[hidden email]>:
-module(wat_clauses).
-export([authenticate_nouns/1]). 
authenticate_nouns([{<<"user_auth">>, _}]) -> 'true';
authenticate_nouns([{<<"user_auth">>, [<<"recovery">>]}]) -> hi;
authenticate_nouns(_Nouns) -> 'false'.
 
In this code (or above), the second clause will never match (because of source-order).
But erlc does not complain about it.

Ferd on IRC mentioned that the compiler might feel free to reorder clauses.
But the call wat_clauses:authenticate_nouns([{<<"user_auth">>, [<<"recovery">>]}])
returns true instead of hi, so no reordering is done.

Dialyzer does not complain either.

Am I right in thinking we should have a warning about a never matching clause?

-- 
Pierre Fenoll


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Anthony Ramine-4
Le 8 avr. 2015 à 08:46, Erik Søe Sørensen <[hidden email]> a écrit :

> It would of course make sense to have the compiler warn in this case.
> However - if my understanding of the compiler implementation is correct - matching of binaries is handled specially, and apparently in such a manner that it doesn't realise that the two binary patterns are identical.
> /Erik

That would be a reasonable explanation, but unfortunately it's not the case. If you compile that module to BEAM assembly, you can see very that the two patterns are indeed merged together, there is only one occurrence of "user_auth" in it.

{function, authenticate_nouns, 1, 2}.
  {label,1}.
    {line,[{location,"wat_clauses.erl",3}]}.
    {func_info,{atom,wat_clauses},{atom,authenticate_nouns},1}.
  {label,2}.
    {test,is_nonempty_list,{f,4},[{x,0}]}.
    {get_list,{x,0},{x,1},{x,2}}.
    {test,is_tuple,{f,4},[{x,1}]}.
    {test,test_arity,{f,4},[{x,1},2]}.
    {get_tuple_element,{x,1},0,{x,3}}.
    {get_tuple_element,{x,1},1,{x,4}}.
    {test,bs_start_match2,{f,4},5,[{x,3},0],{x,5}}.
    {test,bs_match_string,{f,4},[{x,5},72,{string,"user_auth"}]}.
    {test,bs_test_tail2,{f,4},[{x,5},0]}.
    {test,is_nil,{f,3},[{x,2}]}.
    {move,{atom,true},{x,0}}.
    return.
  {label,3}.
    {test,is_nonempty_list,{f,4},[{x,4}]}.
    {get_list,{x,4},{x,6},{x,7}}.
    {test,bs_start_match2,{f,4},8,[{x,6},0],{x,8}}.
    {test,bs_match_string,{f,4},[{x,8},64,{string,"recovery"}]}.
    {test,bs_test_tail2,{f,4},[{x,8},0]}.
    {test,is_nil,{f,4},[{x,7}]}.
    {test,is_nil,{f,4},[{x,2}]}.
    {move,{atom,hi},{x,0}}.
    return.
  {label,4}.
    {move,{atom,false},{x,0}}.
    return.

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: Warning not emitted on string matching clauses

Björn-Egil Dahlberg-2
This is a part of the compiler I'm not really an expert on, I would defer to Robert or Björn in this case.
However, I believe it is the "variable split" that happens in match compilation in kernel that is the cause of this. First I believed it was just the order in which the compiler chooses the values to match (left to right) but i don't think it's the case here (though i've seen that too).

Ideally, we shouldn't traverse the clauses left to right but instead weight and sniff out the optimal matching path in the tree but that's a debate for another time perhaps (and a fun one).

One could also question why the kernel should emit this warning and not the linter. I guess pragmatism.

2015-04-08 9:52 GMT+02:00 Anthony Ramine <[hidden email]>:
Le 8 avr. 2015 à 08:46, Erik Søe Sørensen <[hidden email]> a écrit :

> It would of course make sense to have the compiler warn in this case.
> However - if my understanding of the compiler implementation is correct - matching of binaries is handled specially, and apparently in such a manner that it doesn't realise that the two binary patterns are identical.
> /Erik

That would be a reasonable explanation, but unfortunately it's not the case. If you compile that module to BEAM assembly, you can see very that the two patterns are indeed merged together, there is only one occurrence of "user_auth" in it.

{function, authenticate_nouns, 1, 2}.
  {label,1}.
    {line,[{location,"wat_clauses.erl",3}]}.
    {func_info,{atom,wat_clauses},{atom,authenticate_nouns},1}.
  {label,2}.
    {test,is_nonempty_list,{f,4},[{x,0}]}.
    {get_list,{x,0},{x,1},{x,2}}.
    {test,is_tuple,{f,4},[{x,1}]}.
    {test,test_arity,{f,4},[{x,1},2]}.
    {get_tuple_element,{x,1},0,{x,3}}.
    {get_tuple_element,{x,1},1,{x,4}}.
    {test,bs_start_match2,{f,4},5,[{x,3},0],{x,5}}.
    {test,bs_match_string,{f,4},[{x,5},72,{string,"user_auth"}]}.
    {test,bs_test_tail2,{f,4},[{x,5},0]}.
    {test,is_nil,{f,3},[{x,2}]}.
    {move,{atom,true},{x,0}}.
    return.
  {label,3}.
    {test,is_nonempty_list,{f,4},[{x,4}]}.
    {get_list,{x,4},{x,6},{x,7}}.
    {test,bs_start_match2,{f,4},8,[{x,6},0],{x,8}}.
    {test,bs_match_string,{f,4},[{x,8},64,{string,"recovery"}]}.
    {test,bs_test_tail2,{f,4},[{x,8},0]}.
    {test,is_nil,{f,4},[{x,7}]}.
    {test,is_nil,{f,4},[{x,2}]}.
    {move,{atom,hi},{x,0}}.
    return.
  {label,4}.
    {move,{atom,false},{x,0}}.
    return.

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions


_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions