Re: Proto patch for circular dependencies

[Akim Demaille]

>>>>> "Alexandre" == Alexandre Oliva <address@hidden> writes:

Alexandre> On Oct 12, 2000, Akim Demaille <address@hidden> wrote:
>> So yes, it makes a difference.  I tend to think that, at least for
>> error messages, a bit too much information is OK.

>> What do you think?

Alexandre> Looks good enough to me.  After all, it's just for
Alexandre> debugging :-)

Right.  So here is my proposal.  If you OK it, I'd like to immediately
change AC_EXEEXT and AC_OBJEXT to use _AC_LINK_IFELSE etc., because it
will help us to see the other circular dependencies (if we apply the
patch below, there are something like 80 new failures in the test
suite, nearly all of which being due to AC_PROG_CC).  It should also
help Morten finish his patch.

Index: ChangeLog
from  Akim Demaille  <address@hidden>
        Diagnose AC_REQUIRE circular dependencies.

        * acgeneral.m4 (_AC_DEFUN_PRO, _AC_DEFUN_EPI): Keep a stack of
        macro expansions, _AC_EXPANSION_STACK.
        Use `_AC_EXPANDING(FOO)' to record the fact that `FOO' is being
        expanded.
        (_AC_REQUIRE): Diagnose required macros which are already being
        expanded.
        Record in _AC_EXPANSION_STACK the AC_REQUIRE calls.
        (_AC_EXPANSION_STACK_DUMP): New macro.

2000-10-12  Akim Demaille  <address@hidden>

Index: TODO
===================================================================
RCS file: /cvs/autoconf/TODO,v
retrieving revision 1.90
diff -u -r1.90 TODO
--- TODO 2000/08/09 07:41:52 1.90
+++ TODO 2000/10/12 09:02:29
@@ -9,6 +9,15 @@
 
 * Autoconf 2.50
 
+** More tests
+Check that AC_REQUIRE circular dependencies are caught.
+
+** _AC_EXPANSION_STACK_DUMP
+Where ever AC_FATAL is used, we should call _AC_EXPANSION_STACK_DUMP.
+Try to see if we can use __FILE__ and __LINE__ in the stack: this
+would provide much more accurate error messages (but it might be
+costly...).
+
 ** --target & AC_ARG_PROGRAM
 Shouldn't *any* `program' be installed as `$target_alias-program' even
 if AC_ARG_PROGRAM is not called?  That would be much more predictable.
Index: acgeneral.m4
===================================================================
RCS file: /cvs/autoconf/acgeneral.m4,v
retrieving revision 1.565
diff -u -r1.565 acgeneral.m4
--- acgeneral.m4 2000/10/11 17:55:46 1.565
+++ acgeneral.m4 2000/10/12 09:02:34
@@ -210,6 +210,13 @@
 # needed machinery for `AC_REQUIRE'.  A macro must be AC_DEFUN'd if
 # either it is AC_REQUIRE'd, or it AC_REQUIRE's.
 #
+# Two things deserve attention and are detailed below:
+#  1. Implementation of AC_REQUIRE
+#  2. Keeping tracking of the expansion stack
+#
+# 1. Implementation of AC_REQUIRE
+# ===============================
+#
 # Of course AC_DEFUN AC_PROVIDE's the macro, so that a macro which has
 # been expanded is not expanded again when AC_REQUIRE'd, but the
 # difficult part is the proper expansion of macros when they are
@@ -490,6 +497,53 @@
 #   DUMP:       undefined
 #   BODY:       TEST2a; TEST3; TEST2b; TEST1
 #   diversions: BODY |-
+#
+#
+# 2. Keeping tracking of the expansion stack
+# ==========================================
+#
+# When M4 expansion goes wrong it is often extremely hard to find the
+# path amongst macros that drove to the failure.  What is needed case
+# is the stack of macro `calls'. One could imagine that GNU M4 would
+# maintain a stack of macro expansion, unfortunately it doesn't, so we
+# do it by hand.  This is of course extremely costly, but the help
+# this stack provides is worth it.  Nevertheless to limit the
+# performance penalty this is implemented only for AC_DEFUN'd macros,
+# not for define'd macros.
+# 
+# The scheme is simplistic: each time we enter an AC_DEFUN'd macros,
+# we pushdef its name in _AC_EXPANSION_STACK, and when we exit the
+# macro, we popdef _AC_EXPANSION_STACK.
+# 
+# In addition, we want to use the expansion stack to detect circular
+# AC_REQUIRE dependencies.  This means we need to browse the stack to
+# check whether a macro being expanded is AC_REQUIRE'd.  For ease of
+# implementation, and certainly for the benefit of performances, we
+# don't browse the _AC_EXPANSION_STACK, rather each time we expand a
+# macro FOO we define _AC_EXPANDING(FOO).  Then AC_REQUIRE(BAR) simply
+# needs to check whether _AC_EXPANDING(BAR) is defined to diagnose a
+# circular dependency.
+# 
+# To improve the diagnostic, in addition to keeping track of the stack
+# of macro calls, _AC_EXPANSION_STACK also records the AC_REQUIRE
+# stack.  Note that therefore an AC_DEFUN'd macro being required will
+# appear twice in the stack: the first time because it is required,
+# the second because it is expanded.  We can avoid this, but it has
+# two small drawbacks: (i) the implementation is slightly more
+# complex, and (ii) it hides the difference between define'd macros
+# (which don't appear in _AC_EXPANSION_STACK) and AC_DEFUN'd macros
+# (which do).  The more debugging information, the better.
+
+
+# _AC_EXPANSION_STACK_DUMP
+# ------------------------
+# Dump the expansion stack.
+define([_AC_EXPANSION_STACK_DUMP],
+[ifdef([_AC_EXPANSION_STACK],
+       [m4_errprint([  ]defn([_AC_EXPANSION_STACK]))dnl
+popdef([_AC_EXPANSION_STACK])dnl
+_AC_EXPANSION_STACK_DUMP()],
+       [m4_errprint([  the top level])])])
 
 
 # _AC_DEFUN_PRO(MACRO-NAME)
@@ -497,6 +551,8 @@
 # The prologue for Autoconf macros.
 define([_AC_DEFUN_PRO],
 [AC_PROVIDE([$1])dnl
+pushdef([_AC_EXPANSION_STACK], [$1 is expanded from...])dnl
+pushdef([_AC_EXPANDING($1)], [$1])dnl
 ifdef([_AC_DIVERT_DUMP],
       [AC_DIVERT_PUSH(defn([_AC_DIVERT_DIVERSION]))],
       [define([_AC_DIVERT_DUMP], defn([_AC_DIVERT_DIVERSION]))dnl
@@ -513,12 +569,13 @@
 ifelse(_AC_DIVERT_DUMP, _AC_DIVERT_DIVERSION,
        [undivert(_AC_DIVERT([PREPARE]))dnl
 undefine([_AC_DIVERT_DUMP])])dnl
+popdef([_AC_EXPANSION_STACK])dnl
+popdef([_AC_EXPANDING($1)])dnl
 ])
 
 
 # AC_DEFUN(NAME, EXPANSION)
 # -------------------------
-#
 # Define a macro which automatically provides itself.  Add machinery
 # so the macro automatically switches expansion to the diversion
 # stack if it is not already using it.  In this case, once finished,
@@ -566,7 +623,8 @@
 # ------------------------------------------
 # If NAME-TO-CHECK has never been expanded (actually, if it is not
 # AC_PROVIDE'd), expand BODY-TO-EXPAND *before* the current macro
-# expansion.  Once expanded, emit it in _AC_DIVERT_DUMP.
+# expansion.  Once expanded, emit it in _AC_DIVERT_DUMP.  Keep track
+# of the AC_REQUIRE chain in _AC_EXPANSION_STACK.
 #
 # The normal cases are:
 #
@@ -591,8 +649,14 @@
 #   `extension' prevents `AC_LANG_COMPILER' from having actual arguments that
 #   it passes to `AC_LANG_COMPILER(C)'.
 define([_AC_REQUIRE],
-[ifndef([_AC_DIVERT_DUMP],
-        [AC_FATAL([$0: cannot be used outside of an AC_DEFUN'd macro])])dnl
+[pushdef([_AC_EXPANSION_STACK],
+         [$1 is required by...])dnl
+ifdef([_AC_EXPANDING($1)],
+      [m4_errprint([AC_REQUIRE: circular dependency of $1])dnl
+_AC_EXPANSION_STACK_DUMP()dnl
+m4exit(1)])dnl
+ifndef([_AC_DIVERT_DUMP],
+    [AC_FATAL([AC_REQUIRE: cannot be used outside of an AC_DEFUN'd macro])])dnl
 AC_PROVIDE_IFELSE([$1],
                   [],
                   [AC_DIVERT_PUSH(m4_eval(_AC_DIVERT_DIVERSION - 1))dnl
@@ -603,6 +667,7 @@
                   [],
                   [AC_DIAGNOSE([syntax],
                                [$1 is AC_REQUIRE'd but is not AC_DEFUN'd])])dnl
+popdef([_AC_EXPANSION_STACK])dnl
 ])
 
 
@@ -624,6 +689,8 @@
 
 # AC_PROVIDE(MACRO-NAME)
 # ----------------------
+# Ideally we should use `AC_PROVIDE($1)', but unfortunately many third
+# party macros know that we use `AC_PROVIDE_$1' and they depend on it.
 define([AC_PROVIDE],
 [define([AC_PROVIDE_$1])])