Elixir: Multiple whens in function guard causes unexpected behavior

[elbow-jason]

Summary

When a function is defined with multiple whens in it's guard clause the guard clause is ignored. This leads to very hard to find bugs. When a function is defined with multiple when in it's guard there should be a compile-time error.

A working example: http://elixirplayground.com?gist=31856426c88844505ed455cc18c86ba1

Environment

• Elixir & Erlang versions (elixir --version):

Erlang/OTP 20 [erts-9.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Elixir 1.5.0

• Operating system:

OSX Yosemite 10.10.5

Current behavior

defmodule Foo do

  # has 2 whens
  def broken_guards(number1, number2) when is_number(number1) when is_number(number2) do
    {number1, number2}
  end

  # has 1 when
  def working_guards(number1, number2) when is_number(number1) and is_number(number2) do
    {number1, number2}
  end

end

iex> Foo.broken_guards(1, "not_a_number")
{1, "not_a_number"}
iex> Foo.broken_guards("not_a_number", 1)
{"not_a_number", 1}
iex> Foo.working_guards("not_a_number", 1)
** (FunctionClauseError) no function clause matching in Foo.working_guards/2

    The following arguments were given to Foo.working_guards/2:

        # 1
        "not_a_number"

        # 2
        1

    Attempted function clauses (showing 1 out of 1):

        def working_guards(number1, number2) when is_number(number1) and is_number(number2)

    (someapp) lib/someapp/foo.ex:7: Foo.working_guards/2

Expected behavior

There should be a compile-time error similar to:

** (CompileError) lib/someapp/foo.ex:4: only 1 when is allowed in function guard of broken_guards/2

[lexmag]

It works as expected, please see: https://github.com/elixir-lang/elixir/blob/master/lib/elixir/pages/Guards.md#multiple-guards-in-the-same-clause.

[elbow-jason]

@lexmag I understand that this behavior is documented, but I think this issue should remain open or at least open a line of discussion about it's merits and pitfalls.

This is a poor language design decision and should be removed from Elixir.

The reasons against:

1) It does not add functionality to the language.

2) It does not shorten the source code.

3) It does not add clarity to the intentions of the programmer over using or. It does, in fact, obscure the intentions.

4) It changes the behavior of a reserved word based on the the number of occurrences. The first when is restrictive and the following whens are permissive. In Ecto, the choice was made to allow multiple wheres and that multiple wheres would emulate/alias an AND. This makes sense as it keeps the intentions of the where the same every time it is used; it constrains the query's clauses/results more with each use.

5) It can (and has) lead to very hard to debug code. If you ever accidentally put a when where you meant to put and you will see what I mean. In addition to not behaving anything like and the mistaken when can be very hard to visually identify (because when belongs next to a function definition).

[whatyouhide]

@elbow-jason multiple whens are a language feature. They allow you to write a single head and body for multiple types of arguments. For example, say your function takes two arguments and accepts binaries and charlists:

def my_fun(a, b) when (is_binary(a) and is_binary(b)) or (is_list(a) and is_list(b)) do
  ...
end

The expression can quickly become complex. Using multiple whens you can do

def my_fun(a, b)
    when is_binary(a) and is_binary(b)
    when is_list(a) and is_list(b) do
end

and this means when either they're both binaries or both lists. It works similarly to an or in a guard but it has a small difference: if I do when hd(x) == 1 or is_map(x) the guard will fail if x is not a list (even if it's a map); when using multiple whens, as in when hd(x) == 1 when is_map(x) the guard passes if x is [1 | ...] or if it's a map.

iex(1)> case %{} do
...(1)> x when hd(x) == 1 or is_map(x) -> :first
...(1)> x when hd(x) == 1 when is_map(x) -> :second
...(1)> _other -> :third
...(1)> end
:second

I don't understand 4., the behaviour doesn't change. when simply makes the guard "pass" if any of the whens pass, whatever number of whens you use.

I don't think 5. is a real issue: to me it's similar to saying that using or makes it hard to debug when you meant and because it's easy to gloss over and miss it. 🤔

Does this make sense?

[josevalim]

It does introduce a language feature as it provides failure isolation
between clauses.

And once you rely on those semantics, it can lead to shorter code.

Also the behavior doesn't change based on the number of occurrences. The
first and following when's behave the same: you need at least one to match.

You assumed the operator would behave in a way and it did not. That's no
reason to remove it from the language. :)

Also, before someone brings "the principle of least surprise", I want to
point out it is a poor argument. We all come from different backgrounds and
experiences and those will shape our expectations and surprises.

Since the behavior is clearly documented, then don't see an issue here.
Anyone can still disagree and think it is a poor design decision, but the
feature is not going anywhere.

--

José Valim
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

[elbow-jason]

@josevalim I apologize for my use of strong language irt "poor language design decision". I would take it back if I could. The decisions of this language are fantastic. I should have simply asked to discuss this issue further. We all want to make Elixir better.

After reading the examples from @whatyouhide I see why multiple when clauses were introduced.

However, I would still like to discuss this issue further.

After seeing the examples from @whatyouhide I think I understand the semantics of when better, and if my thoughts are correct each when block is evaluated as a whole pattern; each as a separate guard. Which leads me to my next set of questions.

Shouldn't the or in a function's guard clause result in multiple gaurds for the function? Similar to what when does?

I think if using or were to result multiple guards for a function it would eliminate the need for multiple whens and improve the clarity of the code's intentions.

I realize my questions above once again appeal to the principle of least surprise, but they are actually surprising given my understanding of logical disjunction (or).

In the following example or does not act like logical disjunction it acts like some sort of null operation. It results in ignored, unreachable, dead code (is_map(x)) thus necessitating the need for multiple whens in a guard or additional independent clauses/cases.

iex(1)> case %{} do
...(1)> x when hd(x) == 1 or is_map(x) -> :first
...(1)> x when hd(x) == 1 when is_map(x) -> :second
...(1)> _other -> :third
...(1)> end
:second

I appreciate the feedback. Sorry for being a pain in the butt.

Edit: "clause" => "guard"

[michalmuskala]

To me, multiple guards, simply mean you "folded" multiple bodies together. For example, instead of writing:

def my_fun(a, b)
    when is_binary(a) and is_binary(b) do
  [a | b]
end

def my_fun(a, b)
    when is_list(a) and is_list(b) do
  [a | b]
end

You can "fold" together bodies that are exactly the same:

def my_fun(a, b)
    when is_binary(a) and is_binary(b)
    when is_list(a) and is_list(b) do
  [a | b]
end

Giving that example, I think, makes it clear why the semantics of multiple whens are what they are.

[elbow-jason]

This example more clearly demonstrates the problem with or from above:

    case %{} do
      x when hd(x) == 1 or is_map(x) -> :first
      x when is_list(x) or is_map(x) -> :second
      x when hd(x) == 1 when is_map(x) -> :third
      _other -> :fourth
    end

results in :second which is quite unexpected given the is_map(x) above it in the case statement.

[OvermindDL1]

results in :second which is quite unexpected given the is_map(x) above it in the case statement.

Not really, the first part of hd(x) in the hd(x) == 1 expression will throw an exception (you can test this in iex), which is an early return, 'just' like it would be anywhere else too. :-)

[josevalim]

Shouldn't the or in a function's guard clause result in multiple clauses
for the function? Similar to what when does?

They should not because "or" would then behave differently in guards
compared to everywhere else. An "or" in Elixir does not silently catch
exceptions - nor any guard operator. That's also consistent with how an
operator "or" would work on the majority of languages. A "when" always
catch its exceptions which in turn cause it to fail. So we are being quite

consistent in how "or" and "when" behave throughout the language.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[elbow-jason]

I disagree that or behaves consistently. Consider the following code:

defmodule Example do

  @doc """
  Usage:

    iex> Example.func1(%{})
    :block_5

  In this function `or` behaves as a logical disjunction only in the clause of
  block_5.  The inclusion of `is_list(x) and` in the clause of block_5 changes the
  perceived behavior of the `or` operator. This does not seem like consistent
  behavior for `or`. 

  block_5's clause is matched because in all clauses above 5, `x` is a list,
  there is only one clause to match `x` to, and a list will not match `is_map/1`

  If `or` were to behave like `when` (producing a breakpoint
  in the guard and resulting multiple clauses) then `or` would work
  consistently as demonstrated in `func/2`
  """
  def func1(item) do
    case item do
      [1 | _] = x when is_map(x) ->
        :block_1
      x when (hd(x) == 1 or is_map(x)) ->
        :block_2
      x when (hd(x) == 1) or (is_map(x)) ->
        :block_3
      x when hd(x) == 1 or is_map(item) ->
        if item != x do raise "item was not x somehow" end
        :block_4
      x when is_list(x) and hd(x) == 1 or is_map(x) ->
        :block_5
      x when is_map(x) ->
        :block_6
    end
  end

  @doc """
  In `func2/1` I copied `func/1` and substituted a `when` for the `or` of block_2 to demonstrate
  what should happen when `or` behaves like the operator for logical disjunction.

  Usage: 

    iex> Example.func2(%{})
    :block_2

  """
  def func2(item) do
    case item do
      [1 | _] = x when is_map(x) ->
        :block_1
      x when (hd(x) == 1 when is_map(x)) ->
        :block_2
      x when (hd(x) == 1) or (is_map(x)) ->
        :block_3
      x when hd(x) == 1 or is_map(item) ->
        if item != x do raise "item was not x somehow" end
        :block_4
      x when is_list(x) and hd(x) == 1 or is_map(x) ->
        :block_5
      x when is_map(x) ->
        :block_6
    end
  end

end

[OvermindDL1]

I disagree that or behaves consistently.

Really? Your example shows nothing about the lack of consistency. Compare:

iex(9)> x = nil; hd(x)==1 or is_map(x)
** (ArgumentError) argument error
    :erlang.hd(nil)
iex(9)> x = []; hd(x)==1 or is_map(x)
** (ArgumentError) argument error
    :erlang.hd([])
iex(9)> x = [1]; hd(x)==1 or is_map(x)
true
iex(10)> x = %{}; hd(x)==1 or is_map(x)
** (ArgumentError) argument error
    :erlang.hd(%{})

See how hd(x) throws an exception when x is a map? It is an early return, and acts identically in a guard:

iex(10)> match?(x when hd(x)==1 or is_map(x), nil)
false
iex(11)> match?(x when hd(x)==1 or is_map(x), [])
false
iex(12)> match?(x when hd(x)==1 or is_map(x), [1])
true
iex(13)> match?(x when hd(x)==1 or is_map(x), %{})
false
iex(14)> match?(x when hd(x)==1 when is_map(x), %{})
true

The guard fails in any case where an exception would be thrown, just as it does in normal code. I don't see this inconsistency that you speak of? Each when is becoming its own standalone head, just as it does in Erlang (with ;). If or managed to wrap exceptions then, well, that would be VERY surprising indeed. I would never expect this to do anything but throw an exception:

x = %{}; hd(x)==1 or is_map(x)

If it hides an exception then I would consider the operator fundamentally broken.

[josevalim]

I believe your example is precisely reaffirming what we have been saying so
far: "or" and "when" are distinct, for all the reasons previously said and

reiterated by @overminddl1.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[elbow-jason]

@josevalim @OvermindDL1 is using or in runtime I am addressing compile-time guards.

[elbow-jason]

Ok. I see what you guys are saying. The use is consistent between or in guards and or in execution. I am speaking about or between guards, though.

[elbow-jason]

The fact that one or acts like || given the verity of it's previous expression and the other or does not is not consistent.

[elbow-jason]

However, I would rather have consistency in the syntax/expectations between runtime and compile-time. So I concede the point. Thank you all for your inputs.

Additionally, I hope we can all agree that this clause when hd(x)==1 or is_map(x) is awful and should never be used again.

[whatyouhide]

(it was just a quick example guard, of course it doesn't make really a lot of sense :P)

[OvermindDL1]

Additionally, I hope we can all agree that this clause when hd(x)==1 or is_map(x) is awful and should never be used again.

Optimally dialyzer (or the compiler itself) would warn/error about a no-resolvable guard like that. ^.^;

[josevalim]

The fact that one or acts like || given the verity of it's previous
expression and the other or does not is not consistent.

When does this happen? "or" should always short-circuit, regardless if it
is inside a guard and during execution.

I also wouldn't bring the compile time discussion here, because nothing

here is happening at compile time.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[josevalim]

I also want to add that "or" always fails if any of its operands raises an

error. The handling of the exception is always done elsewhere.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D