Elixir: Assignments from within 'if' statements

Created on 27 Feb 2018 · 3Comments · Source: elixir-lang/elixir

Precheck

Do not use the issues tracker for help or support (try Elixir Forum, Stack Overflow, IRC, etc.)
For proposing a new feature, please start a discussion on the Elixir Core mailing list
For bugs, do a quick search and make sure the bug has not yet been reported
Finally, be nice and have fun!

Environment

Elixir & Erlang versions (elixir --version): Erlang 19.3 release, see below for Elixir version.
Operating system: Ubuntu Linux 16.04.3, Amazon Linux amazonlinux:2017.03.1.20170812

Current behavior

  template_path = ""
  if (config[:app] == :exlam) do
    template_path = Path.join(["./priv", "templates", "#{name}.eex"])
  else
    template_path = Path.join(["#{:code.priv_dir(:exlam)}", "templates", "#{name}.eex"])
  end

Elixir 1.7.0-dev (cbcdcb1623be11e1ff0bd0bd46cb812c563c3bbd), on Ubuntu Linux 16.04.3, template_path is set to the path of the file.
Elixir 1.7.0-dev (4e3e9cefc879b9173461b5a58f766390de9f9989), on Amazon Linux amazonlinux:2017.03.1.20170812, template_path is "".

Fix in code:

  template_path = ""
  template_path = if (config[:app] == :exlam) do
    template_path = Path.join(["./priv", "templates", "#{name}.eex"])
    IO.puts "path is exlam #{template_path}"
    template_path
  else
    template_path = Path.join(["#{:code.priv_dir(:exlam)}", "templates", "#{name}.eex"])
    IO.puts "path is not exlam #{template_path}"
    template_path
  end

Include code samples, errors and stacktraces if appropriate.

Expected behavior

Not sure. Should assignment from within the if be supported? (This was code I found in an existing library.)

Source

cmeiklejohn

Most helpful comment

This is a bug fix two years in the making. :)

This type of assignment, where you assign to a variable inside an if and the value of the variable is available outside, is called "imperative assignment" because the if (or case and similar) would return two values, one which was the value of the if expression and a hidden value that would be assigned to the leaked variables in the outer context.

This programming style makes code harder to understand and hard to refactor, because you can't simply extract the if (or case and similar) to a separate function, you would also have to account for all of the variables that are implicitly changed.

This behaviour also made the language inconsistent. Some constructs would leak variables, such as case, if and friends, but others did not, such as try and fn.

Although we would be able to live with a warning, this behaviour also introduced two bugs:

We could not emit "unused variable" warnings properly for variables assigned in different clauses because we always have to leak them in case they are used later. Even if you were writing code without imperative assignments!
In some situations, we would have to choose between not leaking a variable or breaking tail call semantics:
```
if some_condition? do
  foo(a = 1)
else
  a = 2
  ...
end
```
Although that's a simple example, it can get quite complex in real life. And even if you argue that foo(a = 1) is not a common idiom, remember you could have something like foo(bar()) where bar() is a macro that expands to some code that assigns variables.

For those reasons, we have started warning on this behaviour on v1.3, and now on Elixir v1.7, two years after v1.3.0, we have made the warning an error to address those bugs and also open up the way to further optimize the compiler and fix other complex bugs, such as #7392.

josevalim on 27 Feb 2018

🎉8

All 3 comments

Hmm, it's surprising that it works at all on Ubuntu, actually, since those variables aren't supposed to leak. There is a compiler warning that warns you against doing this, so you should be seeing that compiler warning when compiling the given example. Does that warning appear in either environment?

As the warning says, if you want to make that work correctly you'll need to do this:

template_path =
  if (config[:app] == :exlam) do
    Path.join(["./priv", "templates", "#{name}.eex"])
  else
    Path.join(["#{:code.priv_dir(:exlam)}", "templates", "#{name}.eex"])
  end

devonestes on 27 Feb 2018

Should assignment from within the if be supported?

Intuitively I want to say no, just because the variable = if x do y else z end kind of construct seems to be the "most correct" way to do it, as well as the fact that it's been a compiler warning in the past (see ex. this Stack Overflow question)

queer on 27 Feb 2018

This is a bug fix two years in the making. :)

This behaviour also made the language inconsistent. Some constructs would leak variables, such as case, if and friends, but others did not, such as try and fn.

Although we would be able to live with a warning, this behaviour also introduced two bugs:

We could not emit "unused variable" warnings properly for variables assigned in different clauses because we always have to leak them in case they are used later. Even if you were writing code without imperative assignments!
In some situations, we would have to choose between not leaking a variable or breaking tail call semantics:
```
if some_condition? do
  foo(a = 1)
else
  a = 2
  ...
end
```
Although that's a simple example, it can get quite complex in real life. And even if you argue that foo(a = 1) is not a common idiom, remember you could have something like foo(bar()) where bar() is a macro that expands to some code that assigns variables.