bug#47049: [feature/native] macro-expansion cycle when compiling comp.el

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>>> > I made some local modifications in comp.el, and used that modified
>>>>> > comp.el for a while (by loading it manually at run time) to debug some
>>>>> > problem.  Then I undid those local modifications of comp.el (by saying
>>>>> > "git checkout") and said "make" to rebuild Emacs.  And I see this:
>>>>> >
>>>>> >     ELC      emacs-lisp/comp.elc
>>>>> >   Warning: Eager macro-expansion skipped due to cycle:
>>>>> >       => (load "comp.el") => (macroexpand-all (defalias 
>>>>> > 'comp-add-call-cstr  )) => (macroexpand (comp-loop-insn-in-block  )) => 
>>>>> > (load "comp.el")
>>>>> >
>>>>> > Why does this happen?
>>>>>
>>>>> I'm not sure, but I can reproduce it.
>>>>>
>>>>> I guess (just a guess) it might be because `comp-add-call-cstr' is using
>>>>> a macro `comp-loop-insn-in-block' that is expanding with a
>>>>> `cl-symbol-macrolet' inside? (no idea why this should be a problem).
>>>>>
>>>>> > could it be that while comp.el was modified it
>>>>> > got native-compiled, and now that inconsistent comp.eln gets in the
>>>>> > way?
>>>>>
>>>>> I think this has not to do specifically with native compilation.
>>>>>
>>>>> I see it goes away removing '(setq load-prefer-newer t)' from the
>>>>> invocation tho.
>>>>>
>>>>> Perhaps somebody already more into the macro expansion business might
>>>>> have some suggestion.
>>>>
>>>> Stefan, can you help, please?
>>>
>>> Nothing jumps at me by looking at the code.  I'll have to look at the
>>> actual complete stacktrace I think.
>>
>> Cool didn't know was so easy to generate backtraces to debug this kind
>> of issues.  At the bottom I attached the backtrace I produced and had a
>> look into.
>>
>> IIUC while *loading* "comp.el" we try to advice `macroexpand' and
>> consequentially we try to install a trampoline (was already compiled)
>> for `macroexpand', this is supposed to be done by
>> `comp-subr-trampoline-install' requiring "comp.el" to be loaded again!
>>
>> Assuming my analysis is correct I'm not sure what's the best work around
>> for this condition.  WDYT?
>
> Does the circularity cause problems later on?
> [ As you can see above, this cyclic expansion problem only results in
>   a warning rather than an error: the eager macroexpansion fails, but
>   that just means we fallback on lazy macroexpansion instead.  ]

I'm not aware of any functional problem except the aesthetic of the
warning emitted.

> If so, then let's first focus on solving that problem.
>
> And if not, then it's probably a good idea to understand why the
> problem doesn't show up when the macroexpansion happens lazily:
> is it because we just don't go through that cl-symbol-macrolet while
> compiling the trampoline?

Yes I think that's the case.

> Maybe you can guess my opinion on the best solution: pre-compile all the
> trampoline we may need (and ideally only one trampoline per
> number-of-arguments, more or less)  ;-)

:)

> But maybe a simpler solution is to move the definition of
> `comp-subr-trampoline-install` earlier in the file (before the first
> use of `comp-loop-insn-in-block`)?

Yep that's what I attemped and reported in the other mail, I guess our
mails crossed over the ocean.

Thanks!

  Andrea