Macro & unsafe variable warning

[Fiorillo Nicola]

Hi all,

I have a elixir/phoenix project in production written in 1.2.x elixir version.

I'm migrating to 1.3.1 and some "unsafe variable" warning appear: most of them were real and useful but I cannot understand the following situation:

#!/usr/bin/env elixir

list = [:one]

if false do

  list = [:two | list]

end

IO.inspect list    # warning occurs

If, as warning comment suggests, I write

#!/usr/bin/env elixir

list = [:one]

list =

  if false do

    list = [:two | list]

  end

IO.inspect list

# here list is nil

list variable assumes an incorrect value.

Question is: why elixir compiler assume that block must return a value?

Thanks,

Nicola

[Fiorillo Nicola]

Sorry about the object: macro have not meaning in this post.

[Ben Wilson]

The compiler suggests you re-write it as

list = [:one]

list = if false do

  [:two | list]

else

  list

end

In your case you have no `else` clause, so if the `if` clause fails, it returns nil, which sets `list` to nil.

[Fiorillo Nicola]

Behaviour was clear to me (and logic).

My doubt is: why elixir force me to put a "useless" else clause and lowering readability?

[Ben Wilson]

This has been covered extensively in a number of places, the most definitive being here: https://github.com/elixir-lang/elixir/blob/v1.3/CHANGELOG.md

[Fiorillo Nicola]

You're right: it has been covered extensively but, imho, not deeply discussed.

Actually we meet this situation starting from an our macro implementation.

Here is a POC:

#!/usr/bin/env elixir

defmodule DoSomethingMacro do

  defmacro __using__(_) do

    quote do

      import DoSomethingMacro, only: :macros

    end

  end

 defmacro do_something_on_flag(flag, do: block) when is_boolean(flag) do

    quote do

      case unquote(flag) do

        true  -> unquote(block)

        _     -> IO.puts "do other things"

      end

    end

  end

end

defmodule DoSomethingMacroTest do

  use DoSomethingMacro

  def run() do

    list = [:one]

    do_something_on_flag(true) do

      list = [:two | list] # BTW: why not unsafe variable warning?

    end

    IO.inspect list # unsafe variable warning

    # list is [:two, :one]

    do_something_on_flag(false) do

      list = [:three | list] # unsafe variable warning

    end

    IO.inspect list # unsafe variable warning

    # list is [:two, :one]

    list =

    do_something_on_flag(false) do

      [:fourth | list] # unsafe variable warning

    end

    IO.inspect list

    # list is :ok (due to IO.puts "do other things")

  end

end

DoSomethingMacroTest.run

In our do_something_on_flag (used as a common library) implementation should not assume that block must return something.

When we write

    list =

    do_something_on_flag(false) do

      [:fourth | list] # unsafe variable warning

    end

we break the list variable.

Moreover, if we modify the do_something_on_flag macro in order to manage alse the else clause, we are forced to write something like this

    list =

    do_something_on_flag(false) do

      [:fourth | list]

    else

        list

    end

First: this is useless for us because our macro must apply a standard behaviour in an hypothetical else clause and deny the caller to have a different behaviour.

Second: in my opinion this is an outrage of Elixir elegance and conciseness.

On Thursday, July 14, 2016 at 6:03:04 PM UTC+2, Ben Wilson wrote:

This has been covered extensively in a number of places, the most definitive being here: https://github.com/elixir-lang/elixir/blob/v1.3/CHANGELOG.md

...

[Ben Wilson]

This is the original thread: https://groups.google.com/forum/#!searchin/elixir-lang-core/imperative/elixir-lang-core/3o-omqgCV3E/LQro8PH7EQAJ

You can find another example of discussion here: https://elixirforum.com/t/fix-scope-warning-without-adding-extra-code/1008/25

These go through the logic, as does the original blog post.

I'm not entirely sure what your macro is trying to do, so it's hard to suggest a better pattern. At the moment it looks like it's basically just another `if` like construct, which means that it's going to need to abide by the same rules.