Elixir: `into:` argument to `for` comprehension breaks AST in quote→unquote

[am-kantox]

Environment

$ elixir --version
Erlang/OTP 21 [erts-10.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [hipe]
Elixir 1.7.3 (compiled with Erlang/OTP 21)
$ uname -a
Linux aleksei 4.15.0-38-generic #41-Ubuntu SMP Wed Oct 10 10:59:38 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Current behavior

Consider the following __using__/1 implementation:

defmodule Foo do
  defmacro __using__(args) do
    # AST-based map declaration using Keyword
    kw = for arg <- args, do: {arg, Macro.var(arg, __MODULE__)}
    map_kw = {:%{}, [], kw} # ⇐ hack

    # An attempt to build the map directly using `into:` in call to `for`
    map = for arg <- args, do: {arg, Macro.var(arg, __MODULE__)}, into: %{}

    result =
      quote do
        def foo(unquote(args)) do
          [unquote(map_kw), unquote(map)]
        end
      end

    # printing it out since the attempt to return an invalid AST raises
    result
    |> Macro.to_string()
    |> IO.puts()

    result
  end
end
defmodule Test, do: use Foo, [:aa]

Basically, there are two identical fors building lists out of arguments. The former one builds _the list_, the latter one tries to build _the map_, using into: option in the call to for.

_Currently:_ the former builds the correct map. while the latter raises:

** (CompileError) iex: invalid quoted expression: %{aa: {:aa, [], Foo}}

The standard output from IO.puts:

def(foo([:aa])) do
  [%{aa: aa}, %{aa: {:aa, [], Foo}}]
end

Expected behavior

The results should be the same.

[josevalim]

Hi @am-kantox!

That's because a map is not a quoted expression, it is a value. Every time you have a value, you need to make sure you Macro.escape/1 it so it becomes a AST. We will make sure to improve the error message, because we could absolutely have this information in the error message.

[josevalim]

To clarify, you need to do:

```elixir
quote do
def foo(unquote(args)) do
[unquote(map_kw), unquote(Macro.escape(map))]
end
end

[am-kantox]

Thank you, it makes total sense.

I understand that the documentation on macros is not the first class citizen mostly because we are discouraged to use macros until the understanding is as clear as no documentation is needed at all, but I am positive that the example above would be of a lot of help if placed somewhere. AFAICT, the most comprehensive way of writing the above would be

quote do
  def foo(unquote(args)) do
    [unquote(Macro.escape(kw)), unquote(Macro.escape(map))]
  end
end

and it magically works without Macro.escape/1 for the Keyword only because Keyword is quoted as it’s value itself. Either I am missing something, or everybody but me considers this obvious, but it’s a complete surprise for me that we need Macro.escape/1 for values declared outside of the quote.

[josevalim]

The introductory material on meta-programming covers escaping as the third section, just after quote and unquote: https://elixir-lang.org/getting-started/meta/quote-and-unquote.html

I am not saying it is perfect but it is a topic we do discuss on the entry level material. Documentation can always be improved and we could probably talk about this when we discuss "Quote literals" in the quote documentation and update the documentation for unquote as well to mention it only accepts valid quote ASTs.

[mhanberg]

Hi!

Is this the line of code that outputs the above error? https://github.com/elixir-lang/elixir/blob/741dfda9f4be898c647426493f1d17fb9a4b9c53/lib/elixir/src/elixir_expand.erl#L1119

I am interested in helping out with this issue if work on it hasn't already been started.

Thanks!

[fertapric]

@mhanberg Yes, that's the line where the error message is formatted :)

[eksperimental]

Is this mentioned in the docs in Elixir core?

[am-kantox]

While we are on it, I’d propose another approach. What if we will Macro.escape/1 _all the local variables (hygiened) in the current context_ on quote do call opt-in with an additional argument to quote?

That should not be rather complicated while it probably could make things more readable and uniform:

defmacro foo do
  map = %{foo: 42}
  quote escape_local: true do
    unquote(map)
  end
end

foo() #⇒ %{foo: 42}

/cc @josevalim

[josevalim]

I am not a big fan because you can also have AST in variables. I would
rather call Macro.escape explicitly than add a flag that changes this under
certain circumstances.

If you are working with macros, you need to understand what is a quoted

expression and what isn’t, and being explicit about it is the way to go IMO.

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

[am-kantox]

I agree. Last thought before we bury this proposal 3 feet down: what if the parameter would receive a list of variables to escape:

defmacro foo do
  map = %{foo: 42}
  quote escape: [:map] do
    unquote(map)
  end
end

[josevalim]

I don’t see the benefit over calling unquote(Macro.escape(map)). The flag

makes it look like something especial, but it is a function call only.

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

[michalmuskala]

One thing to consider could be an option that is a fusion of bind_quoted and escaped, something like bind_escaped or similar. I think that could be useful in cases where you want to use bind_quoted, escaping everything manually before-hand is quite tedious. That said, I agree an escaped option alone doesn't give us much.

[am-kantox]

I agree an escaped option alone doesn't give us much.

I agree as well, save for one, not an evident trait: the documentation on function arguments are read hundreds of times more often and thoroughly than guides and pages. That could be a matter of helping newbies to memorize the necessity of escaping.

Also, I am positive bind_escaped is a good consensus.

[josevalim]

The reason bind_quoted exists is because it is very hard to achieve it
manually. You would have to call quote three times. Furthermore, quote and
unquote is what quote takes care of, but not escaping.

If the concern is discoverability, then let’s provide more examples and
improve the docs. We have no plans to change the quote API.

[josevalim]

I have pushed docs in 37149511c324fec4a4676ddf942a161a4bcfd6aa. Also closing in favor of the PR #8336.