Changes focus in require explanation #6041

fxn · 2017-04-27T10:25:17Z

I think from a programmer's perspective, the fact that in order to expand macros the compiler needs to ensure the module is compiled and loaded is not relevant.

The compiler needs the module loaded in memory to have their functions available (a macro is a function prefixed with "MACRO-"), because it may need to call the function early in the expand phase. Note that the module may be loaded in memory when expansion happens:

Integer.digits(1) # function
defmodule M do
  Integer.is_odd(1) # macro
end

by the time is_odd has to be expanded, Elixir.Integer is already loaded in the VM, and thus the macro available for dispatch. But Elixir errs, because the code does not opt-in. Since Elixir knows the macro exists, it gives you a better error message ("you need to require in order to use a macro", or "there is a macro with the same name"), than the one it can give if the module is not even loaded ("unknown function").

From my perspective, that is not really the concern of the programmer. The programmer does not care about what the compiler needs to do. For example, the parallel compiler may need to compile a module if needed as well, and the programmer does not need to say so.

What the programmer cares in my view is that they have to opt-in. This is the spot in the source code where opting-in is stored. And that is a design decision that I suppose has as a goal to prevent macros from leaking everywhere. You need to consciously opt-in.

In order to make the macro available in the scope, Elixir may need to compile and load and therefore require also ensures the module is loaded and compiled, but that is implementation (again, as I see it).

josevalim · 2017-04-27T10:27:34Z

lib/elixir/lib/kernel/special_forms.ex

@@ -507,13 +507,12 @@ defmodule Kernel.SpecialForms do
  defmacro alias(module, opts), do: error!([module, opts])

  @doc """
-  Requires a given module to be compiled and loaded.
+  Tells Elixir you want to use the macros defined in a module.


Maybe: "Requires a module in order to use its macros."

I like @josevalim's suggestion.

If we put on the shoes of an Elixir programmer, what does "Requires" mean?

The structure I have in mind is:

Opt-in to use macros

Expand with example etc.

(Maybe) BTW, Elixir technically needs to compile and load the module if not available (just so you know it in case you are suprised seen the code being run).

The structure looks good to me, but I would mention "requires" just because the term we refer to this process with is "requiring", so mentioning it is not a bad thing IMO.

Hmmm.

I think the docs can use "requires" elsewhere, meaning "what require does". But using that verb in the description of require itself seems circular to me. I don't know what is "requiring" while defining "requiring", you see what I mean.

I see what you mean, but the short line of documentation at the top is meant to just remind you what a function does or give you a slight idea so that you can go to the full docs if you need to use it. We do the same for alias and import btw, where we mention aliasing and importing respectively.

Agree that "telling Elixir" is maybe too informal.

The programmer should know that while public functions are callable, public macros are not unless you opt-in.

require could be renamed to allow_me_to_call_public_macros_from.

Let's see if we can come with a better wording!

@whatyouhide didn't see that last remark before writing my last comment. Good then, I'll revise the patch in a minute.

josevalim · 2017-04-27T10:28:08Z

I agree with the new direction. Maybe we should just discuss what should be the best phrasing. Let's wait for more feedback besides mine before doing further changes. :)

fxn · 2017-04-27T10:47:09Z

Let me add a different angle to the argument.

If opting-in was not necessary, the current explanation would be good. But since opting-in is necessary, from the point of view of the programmer that is all is needed to say. And Elixir knows that in addition to mark the module as required, it has to ensure it is loaded in the VM to make the macro available (in whatever phase). But that is internal.

It could be said, maybe, that Elixir is going to compile and load the required module as a side-effect, in case that matters to the user, but that would be an additional remark, rather than the defining functionality.

josevalim · 2017-04-27T10:52:55Z

Btw, I agree with the premise that knowing the module is loaded or not is irrelevant. :)

whatyouhide · 2017-04-27T11:12:38Z

lib/elixir/lib/kernel/special_forms.ex

-  Notice that usually modules should not be required before usage,
-  the only exception is if you want to use the macros from a module.
-  In such cases, you need to explicitly require them.
+  Exported module functions are globally available, but in order to use


"Exported module functions" reads somehow a bit clunky to me personally. What if we go with

All the functions that a module exports are available everywhere, but in order to use [same as before]

Thoughts?

And we don't talk about exports from the Elixir perspective. I guess we can just say "Modules are globally available".

Mh but it's nice to say that "functions in modules are globally available" IMO. We could say "Public functions in modules are globally available" since we do talk about public/private.

Ahhh, wait. Too much Erlang these days :), in Elixir that would be "public" I guess :).

whatyouhide · 2017-04-27T11:13:13Z

lib/elixir/lib/kernel/special_forms.ex

@@ -507,13 +507,12 @@ defmodule Kernel.SpecialForms do
  defmacro alias(module, opts), do: error!([module, opts])

  @doc """
-  Requires a given module to be compiled and loaded.
+  Tells Elixir you want to use the macros defined in a module.


I like @josevalim's suggestion.

fxn · 2017-04-27T11:55:36Z

All feedback addressed ❤️ .

whatyouhide

Looks beautiful, your PRs are always a pleasure to review and merge 💟

josevalim · 2017-04-27T12:08:15Z

❤️ 💚 💙 💛 💜

eksperimental · 2017-04-27T13:43:53Z

Great addition @fxn
❤️!

josevalim reviewed Apr 27, 2017

View reviewed changes

whatyouhide reviewed Apr 27, 2017

View reviewed changes

changes focus in require explanation

d952e9d

whatyouhide approved these changes Apr 27, 2017

View reviewed changes

josevalim merged commit 3e56f6d into elixir-lang:master Apr 27, 2017

fxn mentioned this pull request Apr 30, 2017

Changes focus in the require explanation elixir-lang/elixir-lang.github.com#979

Merged

Changes focus in require explanation #6041

Changes focus in require explanation #6041

Uh oh!

Conversation

fxn commented Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fxn Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fxn Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josevalim commented Apr 27, 2017

Uh oh!

fxn commented Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josevalim commented Apr 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fxn commented Apr 27, 2017

Uh oh!

whatyouhide left a comment

Choose a reason for hiding this comment

Uh oh!

josevalim commented Apr 27, 2017

Uh oh!

eksperimental commented Apr 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fxn commented Apr 27, 2017 •

edited

Loading

fxn Apr 27, 2017 •

edited

Loading

fxn Apr 27, 2017 •

edited

Loading

fxn commented Apr 27, 2017 •

edited

Loading

eksperimental commented Apr 27, 2017 •

edited

Loading