Elixir: with clause cannot match when case is inside else block

Created on 9 Oct 2017  路  15Comments  路  Source: elixir-lang/elixir

This is very similar to #6380, but shows up when using a case expression inside the else block of a with.

Environment

  • Elixir & Erlang versions (elixir --version): Elixir 1.5.2, Erlang 20.0
  • Operating system: OS X 10.12.6 Sierra

Current behavior

The following code functions correctly, but emits the this clause cannot match because of different types/sizes warning during compilation.

with {:first, int1} when is_integer(int1)   <- {:first, Integer.gcd(2, 4)},
     {:second, int2} when is_integer(int2)  <- {:second, Integer.gcd(2, 4)} do
  {:ok, int1 + int2}
else
  error ->
    case error do
      {:first, nil}  -> {:error, "first number is not integer"}
      {:second, nil} -> {:error, "second number is not integer"}
    end
end

Obviously in this example it's not useful to have an embedded case expression, but I've found it to be useful a number of times for logging an error-specific message and then sharing any amount of cleanup/retry code inside the else block.

Failing test commited to warning_test.exs here: https://github.com/jordan0day/elixir/commit/ff4611acf3f1565f7c4989ade96e24a85ea68860

Expected behavior

No warning should be emitted.

IEx Bug Advanced

Most helpful comment

There are some changes in the erlang compiler around case expressions (in master and PRs) that might make with optimize better. The problem is, it would work nice on OTP 21, but it would work worse then now on previous versions.

All 15 comments

I am not sure if there is much we can do on this case besides maybe moving the logic to a private function. :( To solve this for good, we would need changes in the Erlang compiler as I really don't think we should mark everything in the else block as generated.

/cc @ggcampinho @michalmuskala

There are some changes in the erlang compiler around case expressions (in master and PRs) that might make with optimize better. The problem is, it would work nice on OTP 21, but it would work worse then now on previous versions.

Not a huge showstopper obviously, and wrapping the case bits in another function (even a local anonymous fn) seems to eliminate the compiler warning. Feel free to close the issue unless you'd like to keep it open to reexamine after OTP 21 is released.

I have the impression that is possible to mark the variables generated in the else clause and add generated: true where they are used directly, but it's not so easy to do it.

@ggcampinho it is definitely possible but I really don't think we should do it! :D

Agreed! I also think it's better to wait for OTP 21, maybe put a reference to that issue

Well, we first need to implement this optimization and get it merged for OTP 21 release 馃槅

Hi,
Should this be closed? Also, is there a work-around that won't throw warnings?

A warning still pops up in Elixir 1.7.4 and even 1.8.0-dev compiled on Erlang 21+

warning: this clause cannot match because of different types/sizes

```
@adjustments Application.get_env(:rmas, :adjustments) # Mix.Config adjustments: :check

data =
  case {@adjustments, r.name} do
    {:none, _} ->
      data

    {:check, "MFR100"} ->
      {_data, _adj} = validate("MFR100", data, scheme_list, %{})
      data

    {:check, "MFR700"} ->
      {_data, _adj} = validate("MFR700", data, scheme_list, %{})
      data

    {:adjust, "MFR100"} ->
      {new_data, adj} = validate("MFR100", data, scheme_list, %{})
      adjust_report("MFR153", adj, r.dts, {r.userid, r.batchid})
      new_data
  end

```

@CharlesOkwuagwu what do you mean? This issue is not closed and, as you said, the issue still exists.

@CharlesOkwuagwu I'd say this is a completely separate issue. In this case you're pattern-matching on a compile-time constant value, so the compiler is right to complain some of the branches can never match.

The compiler has a very simple heuristic to decide if it should warn in such cases - it only doesn't warn if the argument of case is a constant. In this case it's only partially constant. I'd say this is more of an issue with the erlang compiler than the elixir layer on top.

@josevalim Sorry, my mistake, I saw the closed from Dialyzer only taking one path inwithstatement #7177.

But yes, the warning still comes up.

@michalmuskala thanks, noted.

why not has Open source in code?

I would like to share my findings in case it helps to future readers.

Elixir's with is internally expanded to a series of nested cases. To give an idea, the code above would be expanded to something similar to:

case {:first, Integer.gcd(2, 4)} do
  {:first, int1} when is_integer(int1) ->
    case {:second, Integer.gcd(2, 4)} do
      {:second, int2} when is_integer(int2) ->
        {:ok, int1 + int2}

      var1 ->
        case var1 do
          error ->
            case error do
              # The Erlang compiler is smart enough to know this will never match.
              {:first, nil}  -> {:error, "first number is not integer"} 
              {:second, nil} -> {:error, "second number is not integer"}
            end

          var2 ->
            error({:with_clause, var2})
        end
    end

  var1 ->
    case var1 do
      error ->
        case error do
          {:first, nil}  -> {:error, "first number is not integer"}
          {:second, nil} -> {:error, "second number is not integer"}
        end

      var2 ->
        error({:with_clause, var2})
    end
end

I believe the warning of this issue is emitted because the Erlang compiler is smart enough to know that the result of the nested case case {:second, Integer.gcd(2, 4)} do would never match{:first, nil}.

One idea to solve this issue is to change the code generated by with, so the the current code is wrapped into a parent case and the results are forwarded to that parent case using :ok/:error tuples. The example above would be then expanded to something like:

case (
  case {:first, Integer.gcd(2, 4)} do
    {:first, int1} when is_integer(int1) ->
      case {:second, Integer.gcd(2, 4)} do
        {:second, int2} when is_integer(int2) ->
          {:ok, (
            {:ok, int1 + int2}
          )}

        err ->
          {:error, err}
      end

    err ->
      {:error, err}
  end
) do
  {:ok, ok} ->
    ok

  {:error, var1} ->
    case var1 do
      error ->
        case error do
          {:first, nil}  -> {:error, "first number is not integer"}
          {:second, nil} -> {:error, "second number is not integer"}
        end

      var2 ->
        error({:with_clause, var2})
    end
end

which does not emit any warning.

Other idea, based on this comment by @nox, is to move the error handling to its own closure:

fun = fn var1 ->
  case var1 do
    error ->
      case error do
        {:first, nil}  -> {:error, "first number is not integer"}
        {:second, nil} -> {:error, "second number is not integer"}
      end

    var2 ->
      error({:with_clause, var2})
  end
end

case {:first, Integer.gcd(2, 4)} do
  {:first, int1} when is_integer(int1) ->
    case {:second, Integer.gcd(2, 4)} do
      {:second, int2} when is_integer(int2) ->
        {:ok, int1 + int2}

      var1 ->
        fun.(var1)
    end

  var1 ->
    fun.(var1)
end

Yes, we want to do the closure but we need to push some improvements to the Erlang compiler to make it more efficient.

Was this page helpful?
0 / 5 - 0 ratings