Skip to content
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

Avoid compile warnings when implementing an empty protocol #11588

Closed

Conversation

QuinnWilton
Copy link
Contributor

Implementing a protocol creates a module that implements the behaviour given by the protocol. A protocol that defines no functions by extension defines no callbacks, and so won't be recognized as a behaviour by the Erlang compiler. This means that Elixir will log a warning when implementing such a protocol, because behaviour_info/1 won't be defined on it, and so Elixir also won't recognize it as a behaviour.

Erlang offers no mechanism to explicitly denote a module as defining a behaviour, but in this case we know that every protocol implementation must define an impl/1 function. By adding a callback that describes this function to all protocols, the warning can be avoided.

Interactive Elixir (1.13.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> defprotocol Empty do
...(1)> end
{:module, Empty,
 <<70, 79, 82, 49, 0, 0, 17, 0, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 2, 77, 0,
   0, 0, 46, 12, 69, 108, 105, 120, 105, 114, 46, 69, 109, 112, 116, 121, 8, 95,
   95, 105, 110, 102, 111, 95, 95, 10, ...>>, {:__protocol__, 1}}
iex(2)> defimpl Empty, for: Integer do
...(2)> end
warning: module Empty is not a behaviour (in module Empty.Integer)
  iex:2: Empty.Integer (module)

This is a strange use-case for protocols, but it shows up in the wild as part of the Witchcraft suite, because typeclasses may extend existing typeclasses without defining their own functions: witchcrafters/type_class#43

There are approaches that I could use to work-around the issue in Witchcraft, but this seems like something that may be worth resolving upstream in Elixir. Feel free to close if you disagree!

@QuinnWilton QuinnWilton force-pushed the quinn/empty-protocol branch 4 times, most recently from 0931078 to 9442858 Compare January 22, 2022 04:15
@@ -143,11 +147,23 @@ defmodule ProtocolTest do
end

test "protocol defines callbacks" do
assert [{:type, 13, :fun, args}] = get_callbacks(@sample_binary, :ok, 1)
assert args == [{:type, 13, :product, [{:user_type, 13, :t, []}]}, {:type, 13, :boolean, []}]
assert [{:type, 17, :fun, args}] = get_callbacks(@sample_binary, :ok, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it alright to modify these tests in this way? It wasn't clear to me how the types in the callback are represented in the abstract format, and whether these values are significant.

@josevalim
Copy link
Member

Thank you @QuinnWilton! I am honestly not sure if we should go down this route because an empty protocol, afaik, has no use in Elixir. But we need to either merge this or add our own warning.

/cc @whatyouhide @ericmj @lexmag @fertapric any thoughts on the matter? :)

@QuinnWilton
Copy link
Contributor Author

QuinnWilton commented Jan 22, 2022

I am honestly not sure if we should go down this route because an empty protocol, afaik, has no use in Elixir.

That makes sense; it's definitely a strange usage pattern! For what it's worth, on the Witchcraft side of things we can likely get around this by explicitly copying the callbacks for any extended type classes into the new type class, so whichever way you decide to go here doesn't impact things in library code too much.

I do think a clearer warning would be good for usability though: likely changing it to an error in a later version.

I can also think of one use case in regular Elixir, but it's possibly a bit contrived. If you define an empty protocol, then you can use it as a sort of extensible variant type, where implementing the protocol adds a type to the variant. Callers can then check whether a given term belongs to the variant, while being able to support new types dynamically, and also query for all such types.

Normally I achieve the same thing by adding a magic __some_function__ to the module, and then searching for all modules that implement that function, but that does require modifying the module itself, and a more ad-hoc approach is sometimes helpful. The empty protocol approach also gives you a SomeProtocol.t() type that will enumerate the implementations, and so will allow you to leverage a little bit more usage out of dialyzer than the runtime enumeration will.

Anyway, I'm just thinking out loud at this point!

@josevalim
Copy link
Member

If you define an empty protocol, then you can use it as a sort of extensible variant type, where implementing the protocol adds a type to the variant.

Yeah, I thought about this but, without a type system, then the check only happens at runtime, and I can't see a reason to check if something has a type. You usually just ask it to execute whatever you want the protocol to do. I don't think Dialyzer would give much here either.

@QuinnWilton
Copy link
Contributor Author

QuinnWilton commented Jan 23, 2022

I don't think Dialyzer would give much here either.

I had a response to this, but after spending the last hour or so trying to convince Dialyzer to fail in the way I'd hoped it would, I think you're probably right here. I was under the mistaken impression that SomeProtocol.t() would give all of the implementations of a protocol, but it actually looks like it's just an alias for term: https://github.com/elixir-lang/elixir/blob/main/lib/elixir/lib/protocol.ex#L891

I've applied the changes you proposed, but I think there's a strong argument to be made for empty protocols not being a useful feature!

Implementing a protocol creates a module that implements the
behaviour given by the protocol. A protocol that defines no
functions by extension defines no callbacks, and so won't be
recognized as a behaviour by the Erlang compiler. This means
that Elixir will log a warning when implementing such a protocol,
because behaviour_info/1 won't be defined on it, and so Elixir
also won't recognize it as a behaviour.

Erlang offers no mechanism to explicitly denote a module as
defining a behaviour, but in this case we know that every protocol
implementation must define an impl/1 function. By adding a callback
that describes this function to all protocols, the warning can be
avoided.
@whatyouhide
Copy link
Member

I tend to cautiously agree that I don't see this as a feature belonging in Elixir itself. @QuinnWilton the use case you described is pretty niche and I think you could still implement it with other tools if you wanted to (such as behaviours). I'm all in favor of a better warning though, of course 😉

@josevalim josevalim closed this in 27c3a4a Apr 26, 2022
@QuinnWilton QuinnWilton deleted the quinn/empty-protocol branch April 26, 2022 21:48
@QuinnWilton
Copy link
Contributor Author

Thank you! The new warning looks good and should make cases like this easier to understand :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants