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

Inline protocol implementations #802

Closed
4 tasks done
josevalim opened this issue Jan 28, 2013 · 7 comments
Closed
4 tasks done

Inline protocol implementations #802

josevalim opened this issue Jan 28, 2013 · 7 comments

Comments

@josevalim
Copy link
Member

Today, a protocol definition and implementation happens as follow:

defprotocol Binary.Chars do
  def to_binary(thing)
end

defimpl Binary.Chars, for: BitString do
  def to_binary(thing) when is_binary(thing) do
    thing
  end
end

When one calls:

Binary.Chars.to_binary "hello"

It is then dispatched to:

Binary.Chars.BitString.to_binary "hello"

I want to allow the following construct as well:

defprotocol Binary.Chars do
  def to_binary(thing)

  defimpl for: BitString do
    def to_binary(thing) when is_binary(thing) do
      thing
    end
  end
end

The example above will inline the to_binary implementation with the protocol one, giving the following advantages:

  1. We skip a remote call for the inlined implementations
  2. We avoid generating .beam for a module

However:

  1. It means a Binary.Chars.BitString is no longer available, so we lose the property that every protocol implementation contains a module representing it (since in this case the implementation is inlined)

If we are proceeding with this task, we should:

  • Provide the inlining implementation
  • Set @moduledoc false for all defimpl for default types
  • Update exdoc to include @only and @except values on protocol pages
  • Deprecate __impl_for__ reflection from protocols (which I don't believe is used)
@yrashk
Copy link
Contributor

yrashk commented Jan 28, 2013

wrt to (1), may be we can still generate the module, but do the inlining anyway?

@josevalim
Copy link
Member Author

@yrashk We could but skipping the .beam file is half of the benefit imo. Just the inlining per se is not worth the amount of work imo.

@rafaelfranca
Copy link
Contributor

What are the implication of not generating the module besides the loss of the property?

@yrashk
Copy link
Contributor

yrashk commented Jan 28, 2013

How about we make .beam file generation... optional (disabled by default for inlined impls?)

@josevalim
Copy link
Member Author

@rafaelfranca I cannot think of anything we would lose. I raised this issue to see if someone can come up with extra drawbacks, including regarding the beam file. :)

@devinus
Copy link

devinus commented Feb 1, 2013

👍

This would allow me to implement Dict with a Protocol and not sacrifice performance.

@josevalim
Copy link
Member Author

Closed in favor #950

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

No branches or pull requests

4 participants