Behaviours, defoverridable and implementations

behaviours
defoverridable
Tags: #<Tag:0x00007f1143143d80> #<Tag:0x00007f1143143c40>

#1

Hi everyone,

One of the features added to Elixir early on to help integration with Erlang code was the idea of overridable function definitions. This is what allowed our GenServer definition to be as simple as:

defmodule MyServer do
  use GenServer
end

Implementation-wise, use GenServer defines functions such as:

def terminate(reason, state) do
  :ok
end

and then mark them as overridable:

defoverridable terminate: 2

As the community grew, defoverridable/1 started to show some flaws in its implementation. Furthermore, the community did not always follow up on best practices, often times marking functions as overridable but without defining a proper Behaviour behind the scenes.

The goal of this proposal is to clarify the existing functionality and propose extensions that will push the community towards best practices.

Using @optional_callbacks

In the example above, we have used defoverridable terminate: 2 to make the definition of the terminate/2 function optional.

However, in some cases, the use of defoverridable seems to be unnecessary. For instance, we provide a default implementation for handle_call/3 and mark it as overridable, but the default implementation simply raises when invoked. That’s counter-intuitive as it would be best to simply not define a default implementation in the first place, truly making the handle_call/3 callback optional.

Luckily, Erlang 18 added support for marking callbacks as optional, which we support on Elixir v1.4. We propose Elixir and libraries to leverage this feature and no longer define default implementations for the handle_* functions and instead mark them as optional.

Instead of the version we have today:

defmodule GenServer do
  @callback handle_call(message, from, state)

  defmacro __using__(_) do
    quote do
      @behaviour GenServer

      def handle_call(_message, _from, _state) do
        raise "handle_call/3 not implemented"
      end

      # ...

      defoverridable handle_call: 3
    end
  end
end

We propose:

defmodule GenServer do
  @callback handle_call(message, from, state)
  @optional_callbacks handle_call: 3

  defmacro __using__(_) do
    quote do
      @behaviour GenServer

      # ...
    end
  end
end

The proposed code is much simpler conceptually since we are using the @optional_callbacks feature instead of defoverridable to correctly mark optional callbacks as optional. defoverridable will still be used for functions such as terminate/2, which are truly required.

For developers using GenServer, no change will be necessary to their code base. The goal is that, by removing unnecessary uses of defoverridable/1, the Elixir code base can lead by example and hopefully push the community to rely less on such tools when they are not necessary.

The @impl annotation

Even with the improvements above, the usage of defoverridable/1 and @optional_callbacks still have one major downside: the lack of warnings for implementation mismatches. For example, imagine that instead of defining handle_call/3, you accidentally define a non-callback handle_call/2. Because handle_call/3 is optional, Elixir won’t emit any warnings, so it may take a while for developers to understand why their handle_call/2 callback is not being invoked.

We plan to solve this issue by introducing the @impl true annotation that will check the following function is the implementation of a behaviour. Therefore, if someone writes a code like this:

@impl true
def handle_call(message, state) do
  ...
end

The Elixir compiler will warn that the current module has no behaviour that requires the handle_call/2 function to be implemented, forcing the developer to correctly define a handle_call/3 function. This is a fantastic tool that will not only help the compiler to emit warnings but will also make the code more readable, as any developer that later uses the codebase will understand the purpose of such function is to be a callback implementation.

The @impl annotation is optional. When @impl true is given, we will also add @doc false unless documentation has been given. We will also support a module name to be given. When a module name is given, Elixir will check the following function is an implementation of a callback in the given behaviour:

@impl GenServer
def handle_call(message, from, state) do
  ...
end

defoverridable with behaviours

While @impl will give more confidence and assistance to developers, it is only useful if developers are defining behaviours for their contracts. Elixir has always advocated that a behaviour must always be defined when a set of functions is marked as overridable but it has never provided any convenience or mechanism to enforce such rules.

Therefore we propose the addition of defoverridable BehaviourName, which will make all of the callbacks in the given behaviour overridable. This will help reduce the duplication between behaviour and defoverridable definitions and push the community towards best practice. Therefore, instead of:

defmodule GenServer do
  defmacro __using__(_) do
    quote do
      @behaviour GenServer
      def init(...) do ... end
      def terminate(..., ...) do ... end
      def code_change(..., ..., ...) do ... end
      defoverridable init: 1, terminate: 2, code_change: 3
    end
  end
end

We propose:

defmodule GenServer do
  defmacro __using__(_) do
    quote do
      @behaviour GenServer
      def init(...) do ... end
      def terminate(..., ...) do ... end
      def code_change(..., ..., ...) do ... end
      defoverridable GenServer
    end
  end
end

By promoting new defoverridable API above, we hope library developers will consistently define behaviours for their overridable functions, also enabling developers to use the @impl true annotation to guarantee the proper callbacks are being implemented.

The existing defoverridable API will continue to work as today and won’t be deprecated.

PS: Notice defoverridable always comes after the function definitions, currently and as well as in this proposal. This is required because Elixir functions have multiple clauses and if the defoverridable came before, we would be unable to know in some cases when the overridable function definition ends and when the user overriding starts. By having defoverridable at the end, this boundary is explicit.

Summing up

This proposal promotes the use the of @optional_callbacks, which is already supported by Elixir, and introduces defoverridable(behaviour_name) which will push library developers to define proper behaviours and callbacks for overridable code.

We also propose the addition of the @impl true or @impl behaviour_name annotation, that will check the following function has been listed as a callback by any behaviour used by the current module.

Feedback?


Questions about Property Testing / Stream Data
Questions about Property Testing / Stream Data
#2

I really like the idea, but I think that @impl might get confused with something related to protocols because of Kernel.defimpl/3, Protocol.assert_impl!/2, and Protocol.extract_impls/2 beeing the only things mentioning “impl” in elixir until now. Therefore I’d opt for something like @override as in Java.


#3

[quote=“josevalim, post:1, topic:3338”]
Therefore we propose the addition of defoverridable BehaviourName, which will make all of the callbacks in the given behaviour overridable. This will help reduce the duplication between behaviour and defoverridable definitions and push the community towards best practice. Therefore, instead of:[/quote]

defmodule GenServer do
  defmacro __using__(_) do
    quote do
      @behaviour GenServer

      def init(...) do ... end
      def terminate(..., ...) do ... end
      def code_change(..., ..., ...) do ... end

      defoverridable init: 1, terminate: 2, code_change: 3
    end
  end
end

We propose:

defmodule GenServer do
  defmacro __using__(_) do
    quote do
      def init(...) do ... end
      def terminate(..., ...) do ... end
      def code_change(..., ..., ...) do ... end
      defoverridable GenServer
    end
  end
end

I think this is a good addition as I’ve incurred into a situation where I had to manually override all callbacks of a certain Behaviour in several modules. However, I’ve got a question and a suggestion:

  • Why does the new example lacks @behaviour GenServer? Does the usage of the new defoverridable GenServer also includes the @behaviour tag?

  • I believe the proposed defoverridable GenServer may cause a certain misdirection to the user since there’s the Module GenServer and the Behaviour GenServer (in this concrete example, but can be applied to other ones I guess). Of course you can’t override Modules, but I believe a more explicit way to inform it is actually the Behaviour that is being overridden could be helpful (specially if my previous question holds).


#4

The reason why we chose @impl is precisely because a protocol is implementation of callbacks. So they are not different, they are exactly the same. So you can think defimpl/3 is about a module of individual @impl.

The reason we rejected @override is because @impl true does not only apply to defoverridable but to any Behaviour. Given when implementing a behaviour without defoverridable there is nothing to override, I don’t think @override would be a good match.

In any case, we are open to discuss alternatives to @impl, but I don’t think @override in particular is a good fit.

Yes but after your questions I realize it can be a source of confusion. I will amend the proposal to not do that.


#5

I think the main reason people are doing crazy macro things around behaviours is that there’s no good guide or example on how to do this.
So when people want to give an interface similar to GenServer with some additional callbacks, they define regular handle_call functions inside __using__ and make things overridable. This has a lot of issues, poor debugability being probably the biggest one.

I think education could go a really long way in here, so I’d like to propose attaching creating a “Write your own behaviour” guide to the proposal.


#7

@michalmuskala You hit the nail on the head, in my opinion. I know Elixir doesn’t strive for being an OO-like language but I think we can all agree some polymorphism is very useful. I’d like to see Elixir be able to do duck-typing like Golang does.

@josevalim To me, defoverridable intuitively means “I am declaring an overridable function/module right here, where I am typing in this source”. It doesn’t sound like “I am implementing a function/module which are overridable and declared somewhere else; here I am just providing their implementation or override of their defaults”. That’s subjective of course.

Is Golang’s transparent duck-typing impossible for Elixir? If my module implements 3 separate (and non-intersecting) function sets, can a function accepting any of those 3 modules transparently recognize my module as any of them, without any statements that my module implements 3rd party overridable functions?

Apologies if it’s a very obvious / dumb question, I admit I am pretty much oblivious about discussions on the mailing list.


#9

I am unsure of what you are asking. If you are talking about data types, Elixir provides both closed ad-hoc polymorphism via pattern matching on function clauses and open ad-hoc polymorphism via protocols (which are close to interfaces). Parametric polymorphism (aka generics) is not a concern, as Elixir is a dynamic language, but that’s not supported by Go anyway.

The other form of polymorphism common to languages is subtype polymorphism but I don’t think that would apply to Elixir or Go.


#10

Yup. We already have a guide on behaviours and we can further extend that. The plan was also to extend the defoverridable implementation to point towards best practices, like backing up your overridable functions with behaviours, which we currently don’t.

Both interpretations are correct. The point is, if you are providing overridable functions because those functions are meant to be called by external modules, which the majority of uses of defoverridable, then it is a best practice to explicitly document the contract you expect the overridable functions to implement, and that’s done with behaviours.


#11

I keep getting mixed up between behaviours and protocols. My mistake. I did additional reading and I am withdrawing my original questions. Apologies! :icon_rolleyes:


#12

I really like these changes, as it really makes it more explicit what is going on.

I do have a question: 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?


#13

In general I like the direction (simplifying and allowing for optional callbacks), but I must admit that @impl true really sticks out as a sore thumb for me. It’s one of those optional things that people are either going to forget, apply only on half the code base, or that certain developers will ignore or over-use. It also separates the behaviour inclusion into different parts in the module.

defmodule Foo do
  use MyBehaviour
  use OtherBehaviour

  @impl true # implemementation for what? MyBehaviour or OtherBehaviour?
  def bar, do: :ok

  @impl true
  def baz(x, y), do: {x, y}

  def qux, do: :ok  # Is this a callback? Is it used?
end

I think it poses the following problems:

  • Denoting which behaviour the implementation is for (one can mix several behaviour implementations in the same module)
  • The behaviour usages is now split out over the module instead of defined in one place
  • It can be forgotten
  • It adds noise to the code

In the end, does it really add a benefit? It just gives some feedback when used with a callback implementation with the same name but different arity, and that’s it. In no other cases is it useful (same arity, different name or mismatch in pattern matching).


#14

I believe you reached the conclusion the use of @impl true is not worth for you (which is fine). :slight_smile:

Misuse and overuse is going to happen pretty much with any feature. For example, the exact same could have been argued about the Registry.

Some extra comments:

I am not sure I follow. The point is exactly to associate the usage of the behaviour with its implementation. The only way to keep it together would be with something like:

impl MyBehaviour do
  def bar, do: :ok
  ...
end

but I believe the above looks… too foreign?

This should be fine because you cannot have duplicated callbacks. Plus the majority of times you are implementing only a single behaviour, so @impl true is less noisy than repeating @impl Foo.Bar.Baz multiple times. For cases you are using multiple behaviours, you can do @impl Foo.Bar.Baz and @impl A.B.C.

Unfortunately we cannot make it required because that would emit too many warnings. However, we can guarantee that, if you have used @impl true once in a module, you have to use it for all callback implementations.

As mentioned in the proposal, it also gives valuable feedback to anyone who is reading the code, especially if the purpose of some particular function is unclear. We can all identify handle_call/3 but how many Phoenix developers would know that an action/3 in a controller is a Phoenix.Controller callback?

Thanks for the feedback!


#15

@josevalim and @eproxus: So how about a shortcut that can emit more warnings and could be much more cleaner?
Simple example:

defmodule MyProject.MyModule do
  impl_behavior MyProject.MyFirstBehavior do
    def first_behavior_method(), do: false
  end

  # ...
end

and this could be interpreted as:

defmodule MyProject.MyModuledo
  use MyProject.MyFirstBehavior

  @impl MyProject.MyFirstBehavior
  def first_behavior_method(), do: false

  # ...
end

In that way we can find forgotten methods, because our implementation is “grouped”, but it’s interpreted still in same module and we do not need repeat any @impl, because shortcut like that could do this automatically.
What do you think about it? Am I forgot about something?

Edit: corrected name, thanks


#16

Ah, TIL :slight_smile:


#17

To be clear, that’s what I tried to proposed above. :slight_smile: Except I used impl instead of override_behaviour. “overriding” a behaviour doesn’t make much sense, you don’t override it, you implement it.

I believe this grouping would be too foreign and create artificial boundaries which we usually don’t do in Elixir. For example, we don’t require explicit boundaries for macros or private stuff. Generally Elixir modules are groups of functions, without extra nesting, and we use anotations such as defp or module attributes to classify some particular functions.


#18

@josevalim: oh, I see your point
hmm, so next idea:

defmodule MyProject.MyModule do
  use MyProject.MyFirstBehavior

  # @impl is false or nil here ...
  # normal methods here ...

  @impl MyProject.MyFirstBehavior
  # all methods for our first behavior

  @impl fasle
  # normal methods
end

This is more strict for developer, but it’s not nested / grouped and it’s still possible to emit more warnings, right?
Is it even possible to do that?


#19

It is likely possible but, again, it is foreign. We don’t use this idiom anywhere else in Elixir.


#20

@josevalim: ok, I see that you’ve already thought about all possible cases.
I don’t have other questions. I think that I understand “your way” little better and also agree with it. :smile:
Thanks for your time!


#21

Ooo, I actually quite like that to be honest… Maybe a defimplement or so? Or defoverride?

/me is not actually suggesting this, just playing with ideas…

Can we have a mix.exs option / switch that enables it to always warning when not used in all cases so we can yell at dependencies that do not implement it properly? ^.^


#22

@josevalim I absolutely second this. I realize it might be confusing and introduce the possibility of inadvertent mistakes but I’ll leave this deliberation to you and the others of the core team.

My take is that having the option to make the compiler more strict is a wanted feature. Some of us just want to get the job done and move on, but others (like me and apparently @OvermindDL1) want to also minimize the possibility of future problems.

My $0.02.