This I don’t quite understand. You’re saying if @impl true shows up before just one of the callbacks, the others will be automatically given warnings? That either doesn’t make sense (you only wanted to implement one of the optional callbacks) or it could be done for all automatically (initiated by the use in the first place).
What I meant was that the intention to use a behaviour is now “shown” in two places, not just one (i.e. first the use and then the @impl. Wether that is a good or bad thing is subjective.
Yeah, I thought about something similar. Would it be possible to do def GenServer.handle_call(...)?
A list of the ideas in this thread so far:
@impl true
def handle_call(message, from, state) do
# ...
end@impl GenServer
def handle_call(message, from, state) do
# ...
end
defimpl handle_call(message, from, state) do
# ...
end
def GenServer.handle_call(message, from, state) do
# ...
end
impl GenServer do
def handle_call(message, from, state) do
# ...
end
end
If you provide @impl true for any of the callback implementations, any other implementation you have in the same module will also require the use of @impl true. For example, if you have handle_call and handle_info and you add @impl true to only one of them, the other will warn. The callbacks you have not explicitly implemented won’t require @impl true.
defmodule GenServer do
@callback handle_call(message, from, state)
@optional_callbacks handle_call: 3
defmacro __using__(_) do
quote do
@behaviour GenServer
def handle_call(msg, from, state) do
IO.puts inspect state
{:ok, state}
end
end
end
end
Would be able to be expressed as this:
defmodule GenServer do
@callback handle_call(message, from, state)
# Or whatever the macro would be called
defoptional_callback handle_call(msg, from, state) do
IO.puts inspect state
{:ok, state}
end
defmacro __using__(_) do
quote do
# Now so terse, it's hard to justify the __using__
@behaviour GenServer
Behaviour.derive GenServer # or w/e syntax for hooking into the macro
end
end
end
That would be awesome!
It might also give us a hook to address some pain points in best practices, via additional capabilities:
Two behaviours have an overlapping interface (ie start_link/1), each with defaults, and you want a module that implements them both:
defmodule Overlap do
@behaviour GenServer
@behaviour AlsoHasStartLink
Behaviour.derive GenServer, except: [start_link: 1]
Behaviour.derive AlsoHasStartLink
end
Default overridable behaviour implementations are a pain to test since they live entirely inside quotes. defoptional_callback helps move them outside the quote and closer to the behaviour callback it implements. Is there a good way it could also make the optional implementation available on the behaviour module itself for testing, so we don’t have to set up dummy test modules to use them into? I can’t think of a good way to do this, but someone cleverer than I might.
So it is only based on the name of the callbacks? What if I add it to one optional callback implementation, and the other is just another function named the same as an optional callback (but with a different arity)? If the programmer just wanted another function that happens to have an overlapping name, it would be a false warning, no?
I still think @impl true adds more confusion that it solves. The name is also too generic in my opinion.
That won’t work. Whatever is calling your code, such as a GenServer, can’t know which one you picked. Conflicting behaviours are always going to be an error.
There seems to be a lot of confusion on this thread related to what is a behaviour and what are overridable functions. You can’t derive a behaviour because a behaviour has no default implementations to derive.
And no, that’s not what is being proposed. Behaviours and protocols are abstract, they don’t provide implementations (and they won’t). This is about making it clear when you are implementing a particular behaviour.
Maybe I misunderstood, but wasn’t the warning supposed to be about a mismatch in arity? If a behaviour exposes the optional callbacks foo/1 and bar/2, and I put an @impl true in front of my foo/1 and I have a bar/3, what would happen?
No, we will warn if you put @impl true before a function and there is no callback with that exact name and arity. It is not about a partial match on the name or arity. And once you use @impl true in a given module, we will require all callbacks to be properly tagged as @impl true.
Can you please re-read the original proposal and try to pinpoint which part may have led you to think it is something specific to a given name or arity, so we can further clarify it?
Gotcha, I think I just read too much into “extend the defoverridable implementation to point towards best practices”; and read “help backing up your overridable functions with behaviours” as "help backing up your behaviours with default overridable functions.
I think we’re on the same page now, and definitely 100% on this proposal.
I think I get it now: If @impl true is used, the function marked will get tested against any existing optional callback and a warning will get issued if none is found. If @impl true is used somewhere, all functions that match an optional callback pattern will get warnings if they themselves don’t use @impl true. Correct me if I’m wrong.
Therefore, if a function exists that does not match the pattern of an optional callback (different name or arity) no warnings would be issued. Which I thought was the case, but I’m not sure from where. I guess it was the combo of “arity mismatch warning” and “other callbacks will get warnings if @impl true is used once”.
Let me re-ask my earlier question because it got overlooked because of the discussion that sparked here directly afterwards :
How can we properly migrate to this new way of doing things, while for the time being keeping our code backwards-compatible with older Elixir versions?Use @optional_callbacks but also write defoverridable for backwards compatibility? Or is there a way to switch between these statements at compile-time based on the Elixir version, to make it very explicit that part of it is legacy-support that might be removed in the future?
The suggestions here should be backwards compatible. For example, imagine we make handle_call/3 an optional callback and we change use GenServer to no longer implement handle_call/3 by default. As long as we change the GenServer implementation to also cope with that, then for the user it makes no difference. If the user code defines its own handle_call/3, then it has nothing to override now and it should just work.
@impl true is not a concern because it applies only to those consuming the behaviour (and not the ones implementing it).
:
