elixir-lang / elixir

Elixir is a dynamic, functional language for building scalable and maintainable applications
https://elixir-lang.org/
Apache License 2.0
24.55k stars 3.38k forks source link

Macro in rescue fails to compile #12209

Closed hissssst closed 2 years ago

hissssst commented 2 years ago

Elixir and Erlang/OTP versions

Erlang/OTP 24 [erts-12.3.2.5] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Elixir 1.13.4 (compiled with Erlang/OTP 24)

Operating system

$ uname -a
Linux 5.15.72 #1-NixOS SMP Wed Oct 5 08:39:44 UTC 2022 x86_64 GNU/Linux

Current behavior

File bug.ex is

defmodule Bug do
  defmacrop argerr(e) do
    quote(do: unquote(e) in ArgumentError)
  end

  def f(x) do
    try do
      x
    rescue
      argerr(e) ->
        e
    end
  end
end
$ elixir bug.ex
warning: variable "e" does not exist and is being expanded to "e()", please use parentheses to remove the ambiguity or change the variable name
  bug.ex:10: Bug.f/1

** (CompileError) bug.ex:10: invalid "rescue" clause. The clause should match on an alias, a variable or be in the "var in [alias]" format
    (stdlib 3.17.2.1) lists.erl:1358: :lists.mapfoldl/3
    (stdlib 3.17.2.1) lists.erl:1358: :lists.mapfoldl/3
    (stdlib 3.17.2.1) lists.erl:1359: :lists.mapfoldl/3

Expected behavior

Macro is expanded properly.


Originally, I came by this error when I've found out that there is no suitable Macro.Env.context for expanding parts of AST which are meant to be used as rescue clauses

hissssst commented 2 years ago

Plus there is no documentation for this behaviour. So, I'd expect a fix or at least some notice in documentation that macros are not allowed in rescue clauses

Ljzn commented 2 years ago

It seems because macro won't be expanded inside the -> clause.

defmodule A do
  defmacro m do
    quote do
      :expanded
    end
  end
end
import A

quote do
  m()
end
|> Macro.expand(__ENV__)

# :expanded
import A

quote do
  m() -> 1
end
|> Macro.expand(__ENV__)

# [{:->, [], [[{:m, [context: Elixir, imports: [{0, A}]], []}], 1]}]
import A

quote do
  a -> m()
end
|> Macro.expand(__ENV__)

# [{:->, [], [[{:a, [], Elixir}], {:m, [context: Elixir, imports: [{0, A}]], []}]}]
josevalim commented 2 years ago

@hissssst macros are allowed either on the left side or the right side of in, but not in in. I dont think we can address this otherwise without making in a special form.

hissssst commented 2 years ago

macros are allowed either on the left side or the right side of in, but not in in.

Yes, I can see that. Perhaps we can add new context to Macro.Env or make in a special form (because it behaves like special form).

Otherwise, we can come across some strange bugs, like

defmodule SampleTest do
  use ExUnit.Case
  doctest Sample

  test "greets the world" do
    q =
      quote do
        try do
          x
        rescue
          e in ArgumentError -> e
        end
      end

    assert (quote do
        try do
          x
        rescue
          e in ArgumentError -> e
        end
      end = q)
  end
end

Results in

$ mix test

== Compilation error in file test/sample_test.exs ==
** (ArgumentError) invalid right argument for operator "in", it expects a compile-time proper list or compile-time range on the right side when used in guard expressions, got: ArgumentError
    (elixir 1.13.4) lib/kernel.ex:4245: Kernel.raise_on_invalid_args_in_2/1
    (elixir 1.13.4) expanding macro: Kernel.in/2
    test/sample_test.exs:19: SampleTest."test greets the world"/1
    (ex_unit 1.13.4) expanding macro: ExUnit.Assertions.assert/1
    test/sample_test.exs:15: SampleTest."test greets the world"/1
josevalim commented 2 years ago

or make in a special form (because it behaves like special form).

It is not really behaving like a special form. Any macro has control over what it may expand on or rewrite or keep as is, which is what try is doing.

Otherwise, we can come across some strange bugs, like

That's a separate bug (assert is not considering quote is a special form). Luckily easier to fix, I will push a fix soon. :)

hissssst commented 2 years ago

Yeah, but this kind of limitation is strange and it is not difficult to fix. I don't know what do you mean by special form, I just think that in behaves differently in three situations: in regular context, in pattern and now inside the rescue clause pattern too. So, I think it would be nice to add a way to determine the context in compile time (line Macro.Env.context)

And try is not a macro, it is a special form. Macro which expands once (or expands until x in Exception is found), would behave correctly in this situation, but try is trying to expand the macro with the wrong Macro.Env.context (because there is no right Macro.Env.context for this situation)

I'd suggest adding :exception_match to the Macro.Env.context and modifying Macro.Env.in_match? to return true when the context is an :exception_match, plus adding one more function like Macro.Env.in_rescue? or something like this.

Or I'd suggest to modify try to recursively expand rescue clause, until x in Exception occurs or code can't be expanded anymore

hissssst commented 2 years ago

@josevalim , see https://github.com/elixir-lang/elixir/commit/80f24b2bf83d421f908c6409bc7aad20be8926c8#commitcomment-87503006

josevalim commented 2 years ago

I don't know what do you mean by special form, I just think that in behaves differently in three situations: in regular context, in pattern and now inside the rescue clause pattern too.

The thing about macros is that in can behave in 1000 different ways.

defmodule MyMacro do
  defmacro new_add({:in, _, [x, y]}) do
    quote do
      unquote(x) + unquote(y)
    end
  end
end

And then:

import MyMacro
new_add 1 in 3
#=> 4

My point is that you are trying to say this behaviour is caused or a responsibility of in, but the behaviour is caused by the parent macro.

Or I'd suggest to modify try to recursively expand rescue clause, until x in Exception occurs or code can't be expanded anymore

Yes, this is most likely the appropriate fix!

hissssst commented 2 years ago

You're right, I was not completely correct. Let me add an important detail to explain what I've actually meant

After expansion of all user-defined macros, in behaves differently in three different contexts: in regular context as Enum.member, in pattern as a orelse chain, in rescue as a special syntax inside special form.

hissssst commented 2 years ago

My point is that you are trying to say this behaviour is caused or a responsibility of in, but the behaviour is caused by the parent macro.

You're almost right, but try is not a macro and it can't be expanded

josevalim commented 2 years ago

The fact try cannot be expanded or not does not change my explanation. The point is still the same: the behaviour of in is given by the macro/special form, not in. The same way that the special form quote changes x in y to mean {:in, [], [x, y]}.

hissssst commented 2 years ago

Yes, that's true

hissssst commented 2 years ago

:heart: