Skip to content

Add local_for_callback option to Macro.Env.expand_import #14620

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions lib/elixir/lib/macro/env.ex
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ defmodule Macro.Env do
]

@type expand_import_opts :: [
allow_locals: boolean(),
allow_locals: boolean() | (Macro.metadata(), atom(), arity(), [atom()], t() -> any()),
check_deprecations: boolean(),
trace: boolean()
]
Expand Down Expand Up @@ -555,8 +555,17 @@ defmodule Macro.Env do
## Options
* `:allow_locals` - when set to `false`, it does not attempt to capture
local macros defined in the current module in `env`
* `:allow_locals` - controls how local macros are resolved.
Defaults to `true`.
- When `false`, does not attempt to capture local macros defined in the
current module in `env`
- When `true`, uses a default resolver that looks for public macros in
the current module
- When a function, uses the function as a custom local resolver. The function
must have the signature: `(meta, name, arity, kinds, env) -> function() | false`
where `kinds` is a list of atoms indicating the types of symbols being
searched (e.g., `[:defmacro, :defmacrop]`)
* `:check_deprecations` - when set to `false`, does not check for deprecations
when expanding macros
Expand All @@ -580,13 +589,27 @@ defmodule Macro.Env do
trace = Keyword.get(opts, :trace, true)
module = env.module

# When allow_locals is a callback, we don't need to pass module macros as extra
# because the callback will handle local macro resolution
extra =
case allow_locals and function_exported?(module, :__info__, 1) do
true -> [{module, module.__info__(:macros)}]
false -> []
if is_function(allow_locals, 5) do
[]
else
case allow_locals and function_exported?(module, :__info__, 1) do
true -> [{module, module.__info__(:macros)}]
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be moved inside the local_for_callback function, threfore we don't need to pass either extra or allow_locals as argument to elixir_dispatch.expand_import.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you like me to remove extra argument from the dispatch call and instead pass a resolver callback using module.__info__(:macros) or macro_exported??

Copy link
Member

Choose a reason for hiding this comment

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

If it is possible, yes, it would be perfect.

false -> []
end
end

case :elixir_dispatch.expand_import(meta, name, arity, env, extra, allow_locals, trace) do
case :elixir_dispatch.expand_import(
meta,
name,
arity,
env,
extra,
allow_locals,
trace
) do
{:macro, receiver, expander} ->
{:macro, receiver, wrap_expansion(receiver, expander, meta, name, arity, env, opts)}

Expand Down
8 changes: 7 additions & 1 deletion lib/elixir/src/elixir_dispatch.erl
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,13 @@ expand_import(Meta, Name, Arity, E, Extra, AllowLocals, Trace) ->
do_expand_import(Dispatch, Meta, Name, Arity, Module, E, Trace);

_ ->
Local = AllowLocals andalso elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E),
Local = case AllowLocals of
false -> false;
true -> elixir_def:local_for(Meta, Name, Arity, [defmacro, defmacrop], E);
Fun when is_function(Fun, 5) ->
%% If we have a custom local resolver, use it.
Fun(Meta, Name, Arity, [defmacro, defmacrop], E)
end,

case Dispatch of
%% There is a local and an import. This is a conflict unless
Expand Down