Skip to content

Macro in rescue fails to compile #12209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hissssst opened this issue Oct 20, 2022 · 13 comments
Closed

Macro in rescue fails to compile #12209

hissssst opened this issue Oct 20, 2022 · 13 comments

Comments

@hissssst
Copy link
Contributor

hissssst commented Oct 20, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

Ljzn commented Oct 21, 2022

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
Copy link
Member

@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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@josevalim
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

Yes, that's true

@hissssst
Copy link
Contributor Author

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants