Elixir: super in overridable macros does not work

[michalmuskala]

Environment

• Elixir version (elixir -v):

Erlang/OTP 18 [erts-7.3] [source] [64-bit] [smp:8:8] [async-threads:10] [hipe] [kernel-poll:false]

IEx 1.4.0-dev (ae27125)

Current behavior

Given the code below:

defmodule Test do
  defmacro foo(opts) do
    quote do
      unquote(opts)
    end
  end

  defoverridable foo: 1

  defmacro foo(opts) do
    super(opts)
  end
end

It compiles with a warning, and when used produces an error:

$ iex -r test.exs
warning: no clause will ever match
  test.exs:2

iex(1)> require Test
Test
iex(2)> Test.foo(1)
** (FunctionClauseError) no function clause matching in Test."foo (overridable 1)"/1
    test.exs:2: Test."foo (overridable 1)"(1)
    expanding macro: Test.foo/1
    iex:2: (file)

Expected behavior

I can use super in macros marked as overridable.

[whatyouhide]

I'm looking into this.

[whatyouhide]

New notes and discoveries:

• we should look into what happens when I have an overridable function and try to override it with a macro and viceversa
• private macros are nastier because we don't generate exported functions for those, and right now defoverridable alone breaks private macros:

defmodule Test do
  defmacrop priv_macro(), do: "first"
  defoverridable priv_macro: 0
  def my_fun(), do: priv_macro()
end

## ** (CompileError) my_file.ex:4: undefined function priv_macro/0
##    (stdlib) lists.erl:1337: :lists.foreach/2
##    (elixir) lib/code.ex:363: Code.require_file/2
##    (elixir) lib/enum.ex:1185: Enum."-map/2-lists^map/1-0-"/2

[lexmag]

Considering overriding of private macros never worked before maybe we should disallow private macros overriding.
I suppose it would be good to disallow private functions overriding in general, it seems magical to me to have overriding for private stuff, if overriding is needed it's likely a public thing. :bowtie:

[danhper]

I think overriding private functions make sense to override a part of the internal behavior
of a module.
A real world example would be HTTPoison, which let the user
override functions which should probably not be public, for example the logic to parse the response body.

[whatyouhide]

@tuvistavie HTTPoison could use a few other less magic methods to achieve that IMO, such as use HTTPoison, headers_function: :my_function or similar. I am 👍 on not supporting private macros overriding since it already doesn't work, and for the future I think private functions should go too.

[lexmag]

HTTPoison is the first thing that came to my mind, and I think all these overridable functions can be public since users do need somehow to know about their existence. And as @whatyouhide said there could be less magical way.

[danhper]

Now you say it, I agree it could be easily achieved in a less magical way,
and I cannot think of any other valid use case, so :+1: on this!

[josevalim]

I am OK with not making private functions overridable. We will need to emit
warnings for now and leave a TODO for us to fix it later.

On Wednesday, June 22, 2016, Daniel Perez [email protected] wrote:

Now you say it, I agree it could be easily achieved in a less magical way,
and I cannot think of any other valid use case, so 👍 on this!

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/elixir-lang/elixir/issues/4830#issuecomment-227704959,
or mute the thread
https://github.com/notifications/unsubscribe/AAAlboyR5QPRvFo5ke_4Et2aXUpMqIbrks5qOQ6FgaJpZM4I5NYE
.

_José Valim_
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

[whatyouhide]

Yes, on it.

On Wednesday, 22 June 2016, José Valim [email protected] wrote:

I am OK with not making private functions overridable. We will need to emit
warnings for now and leave a TODO for us to fix it later.

On Wednesday, June 22, 2016, Daniel Perez <[email protected]

Now you say it, I agree it could be easily achieved in a less magical
way,
and I cannot think of any other valid use case, so 👍 on this!

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<
https://github.com/elixir-lang/elixir/issues/4830#issuecomment-227704959>,
or mute the thread
<
https://github.com/notifications/unsubscribe/AAAlboyR5QPRvFo5ke_4Et2aXUpMqIbrks5qOQ6FgaJpZM4I5NYE

.

_José Valim_
www.plataformatec.com.br
Skype: jv.ptec
Founder and Director of R&D

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/elixir-lang/elixir/issues/4830#issuecomment-227727586,
or mute the thread
https://github.com/notifications/unsubscribe/ADtcSoSPG8SbtDS5VC9WzkoFsN5Di-Brks5qOSpMgaJpZM4I5NYE
.

Andrea Leopardi
an.[email protected]

[edgurgel]

Oh Hi! I agree HTTPoison & HTTPotion abuse a bit of private overridable functions and this was my tentative to expose a better interface: https://github.com/edgurgel/httpehaviour .
It's not as easy to use but it's not "nasty".

[whatyouhide]

Closing this as the original issue (macros not being overridable) is fixed, and we took care of private macros as well. As for deprecating the ability to make private functions overridable, we can discuss that in #4872 as it's unrelated to this issue anyways :)