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

refine overridable functions when using module Base. #110

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nanne007
Copy link

@nanne007 nanne007 commented Jan 6, 2016

The defoverridable Module.definitions_in(__MODULE__) can cause some unexpected behaviors.
Such as, when defining struct in the module using HTTPoison.Base,
the __struct__ function will be lazily defined,
which causes %__MODULE__{} cannot function correctly.

defmodule FooBar do
  defmacro __using__(_) do
    quote do
      defoverridable Module.definitions_in(__MODULE__)
      Module.definitions_in(__MODULE__) |> IO.inspect
    end
  end
end

defmodule MyModule do
  defstruct bar: nil
  use FooBar
  def foo(%__MODULE__{bar: bar}), do: bar
end

# ====>
# ** (CompileError) test.ex:17: MyModule.__struct__/0 is undefined, cannot expand struct MyModule
#    (elixir) src/elixir_map.erl:58: :elixir_map.translate_struct/4
#    (stdlib) lists.erl:1353: :lists.mapfoldl/3

The `defoverridable Module.definitions_in(__MODULE__)` can cause some unexpected behaviors.
Such as, when defining struct in the module using `HTTPoison.Base`,
 the `__struct__ ` function will be lazily defined,
which causes `%__MODULE__{}` cannot function correctly.
@nanne007
Copy link
Author

nanne007 commented Jan 7, 2016

I just found out that one can use FooBar first to prevent the situation.

defmodule FooBar do
  defmacro __using__(_) do
    quote do
      defoverridable Module.definitions_in(__MODULE__)
      Module.definitions_in(__MODULE__) |> IO.inspect
    end
  end
end

defmodule MyModule do
  use FooBar
  defstruct bar: nil
  def foo(%__MODULE__{bar: bar}), do: bar
end

So I just close the PR.

@nanne007 nanne007 closed this Jan 7, 2016
@nanne007 nanne007 deleted the fix branch January 7, 2016 13:44
@edgurgel
Copy link
Owner

edgurgel commented Jan 8, 2016

I think this is an actual bug and it should not be order dependent. Do you mind reopening the PR (if still available)?

@nanne007 nanne007 restored the fix branch January 8, 2016 01:17
@nanne007 nanne007 reopened this Jan 8, 2016
@nanne007
Copy link
Author

nanne007 commented Jan 8, 2016

@edgurgel Here it is.

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

Successfully merging this pull request may close these issues.

2 participants