Proposal: Put macro definitions in their own section at the top of files

[shosti]

The Emacs byte-compiler has some really annoying gotchas around definition order. For instance, the following will cause runtime errors when byte-compiled, but won't usually show up when you're developing since the macro will probably already have been evaluated:

(defun my-function () 
  (my-macro ...))

(defmacro my-macro (...) ...)

In order to avoid this problem, it seems like best practice to always define macros before functions, perhaps in their own section. I guess we could have some broader guidelines around library organization as well (e.g. defcustoms first, then macros, then functions or something along those lines).

[Malabarba]

The entire ordering of a package file is an important topic. Here are a few guidelines that come to mind.

1. Section by functionality, where the first section is reserved for things like version number, defgroup, and widely used macros.
2. Start sections with a page break ^L and a headline comment ;;;.
3. Order inside each section by vars, then macros, then defuns, then mode definitions. Making sure the macros inside a section aren't used anywhere else.

[dgutov]

The presently described issue is alleviated by byte-compilation.

Using it, Flycheck gives a warning:

macro `my-macro' defined too late.

Shouldn't that be enough?

[shosti]

I just tried out the following in its own file:

(defun foo ()
  (bar))

(defmacro bar ()
  3)

I'm not getting the flycheck warning, and byte-compilation proceeds without warning (on Emacs 24.4). The resulting byte-compiled forms:

(defalias 'foo #[nil "\300 \207" [bar] 1])
(defalias 'bar '(macro . #[nil "\300\207" [3] 1]))

Which is invalid, because foo tries to call bar as a function. Maybe the warnings have been added since 24.4?

I've seen this issue enough in the wild (especially when upgrading popular packages) that I think it's worth mentioning.

[dgutov]

Maybe the warnings have been added since 24.4?

Huh, guess so. I tried that in the current master. 24.4 indeed doesn't show the warning.

[swsnr]

I think we should generally recommend to define anything before its first use, be that variables, functions, constants, macros, whatever…

[purcell]

I think we should generally recommend to define anything before its first use, be that variables, functions, constants, macros, whatever…

Yes, absolutely.

[fbergroth]

What about macros using functions from the same package? I've ran into the following weird case:

(defun a ())

(defmacro b ()
  (a))

(defun c ()
  ;; Error: Symbol's function definition is void: a
  (b))

What's going on? Should a and b be put inside eval-and-compile?

[dgutov]

@fbergroth Works for me, no error.

[fbergroth]

@dgutov For me, evaluating it works, but byte compiling it throws an error.

[swsnr]

@fbergroth With this example?! What error do you get, in emacs -Q?

[dgutov]

Huh, right, I see the error when byte-compiling, too. (c) still works, though.

[fbergroth]

@lunaryorn With dummy.el containing the above example, I get:

$ emacs -Q --batch -f batch-byte-compile dummy.el

In toplevel form:
dummy.el:6:1:Error: Symbol's function definition is void: a

[Malabarba]

Yes, that happens because compiling doesn't actually evaluate defuns. For instance, when you byte-compile a .el file, the functions from it are not defined. Therefore, when you byte-compile that example, the macro is expanded, calling a function which is not defined.

I'd consider it a bug, since the byte-compiler does know about the a defun, it's just not using that information. (on second thought, it's a little more complicated than that)

One workaround is to do:

(eval-and-compile
  (defun a ()))

[Malabarba]

As for the actual topic of the issue, I stand by the items 1 and 2 listed on my first comment, but item 3 can be replaced with just "Make sure everything's defined before it is used."

[fbergroth]

It's a tricky error to spot. Maybe add a guideline to wrap defuns used by macros in eval-and-compile (and in turn, defuns used by those defuns...)? Essentially doing the job of the compiler.

[swsnr]

@fbergroth Ah, I didn't see that (a) wasn't quoted. It's not the compiler's job to evaluate defuns, though…

If your macros heavily rely on functions defined in your own package, move these functions into a separate file, and require that.