- Rework secondary expansion so we only defer it if there's a possibility

  it might be needed: for most situations we parse prereqs immediately as
  we used to.  Reduces memory usage.
- Fixes Savannah bug #18622.

1 files changed, 205 insertions, 176 deletions

diff --git a/file.c b/file.c
index 4e3bece..9e744b7 100644
--- a/file.c
+++ b/file.c
@@ -411,11 +411,13 @@ remove_intermediates (int sig)
     }
 }
 
+/* Given a string containing prerequisites (fully expanded), break it up into
+   a struct dep list.  Enter each of these prereqs into the file database.
+ */
 struct dep *
-parse_prereqs (char *p)
+split_prereqs (char *p)
 {
-  struct dep *new = (struct dep *)
-    parse_file_seq (&p, sizeof (struct dep), '|', NULL, 0);
+  struct dep *new = PARSE_FILE_SEQ (&p, struct dep, '|', NULL, 0);
 
   if (*p)
     {
@@ -424,8 +426,7 @@ parse_prereqs (char *p)
       struct dep *ood;
 
       ++p;
-      ood = (struct dep *)
-        parse_file_seq (&p, sizeof (struct dep), '\0', NULL, 0);
+      ood = PARSE_FILE_SEQ (&p, struct dep, '\0', NULL, 0);
 
       if (! new)
         new = ood;
@@ -444,6 +445,85 @@ parse_prereqs (char *p)
   return new;
 }
 
+/* Given a list of prerequisites, enter them into the file database.
+   If STEM is set then first expand patterns using STEM.  */
+struct dep *
+enter_prereqs (struct dep *deps, const char *stem)
+{
+  struct dep *d1;
+
+  if (deps == 0)
+    return 0;
+
+  /* If we have a stem, expand the %'s.  We use patsubst_expand to translate
+     the prerequisites' patterns into plain prerequisite names.  */
+  if (stem)
+    {
+      const char *pattern = "%";
+      char *buffer = variable_expand ("");
+      struct dep *dp = deps, *dl = 0;
+
+      while (dp != 0)
+        {
+          char *percent;
+          int nl = strlen (dp->name) + 1;
+          char *nm = alloca (nl);
+          memcpy (nm, dp->name, nl);
+          percent = find_percent (nm);
+          if (percent)
+            {
+              char *o;
+
+              /* We have to handle empty stems specially, because that
+                 would be equivalent to $(patsubst %,dp->name,) which
+                 will always be empty.  */
+              if (stem[0] == '\0')
+                {
+                  memmove (percent, percent+1, strlen (percent));
+                  o = variable_buffer_output (buffer, nm, strlen (nm) + 1);
+                }
+              else
+                o = patsubst_expand_pat (buffer, stem, pattern, nm,
+                                         pattern+1, percent+1);
+
+              /* If the name expanded to the empty string, ignore it.  */
+              if (buffer[0] == '\0')
+                {
+                  struct dep *df = dp;
+                  if (dp == deps)
+                    dp = deps = deps->next;
+                  else
+                    dp = dl->next = dp->next;
+                  free_dep (df);
+                  continue;
+                }
+
+              /* Save the name.  */
+              dp->name = strcache_add_len (buffer, o - buffer);
+            }
+          dp->stem = stem;
+          dp->staticpattern = 1;
+          dl = dp;
+          dp = dp->next;
+        }
+    }
+
+  /* Enter them as files, unless they need a 2nd expansion.  */
+  for (d1 = deps; d1 != 0; d1 = d1->next)
+    {
+      if (d1->need_2nd_expansion)
+        continue;
+
+      d1->file = lookup_file (d1->name);
+      if (d1->file == 0)
+        d1->file = enter_file (d1->name);
+      d1->staticpattern = 0;
+      d1->name = 0;
+    }
+
+  return deps;
+}
+
 /* Set the intermediate flag.  */
 
 static void
@@ -458,166 +538,94 @@ static void
 expand_deps (struct file *f)
 {
   struct dep *d;
-  struct dep *old = f->deps;
+  struct dep **dp;
   const char *file_stem = f->stem;
-  unsigned int last_dep_has_cmds = f->updating;
   int initialized = 0;
 
   f->updating = 0;
-  f->deps = 0;
 
-  for (d = old; d != 0; d = d->next)
+  /* Walk through the dependencies.  For any dependency that needs 2nd
+     expansion, expand it then insert the result into the list.  */
+  dp = &f->deps;
+  d = f->deps;
+  while (d != 0)
     {
-      struct dep *new, *d1;
       char *p;
+      struct dep *new, *next;
+      char *name = (char *)d->name;
 
-      if (! d->name)
-        continue;
-
-      /* Create the dependency list.
-         If we're not doing 2nd expansion, then it's just the name.  We will
-         still need to massage it though.  */
-      if (! d->need_2nd_expansion)
+      if (! d->name || ! d->need_2nd_expansion)
         {
-          p = variable_expand ("");
-          variable_buffer_output (p, d->name, strlen (d->name) + 1);
-          p = variable_buffer;
+          /* This one is all set already.  */
+          dp = &d->next;
+          d = d->next;
+          continue;
         }
-      else
-        {
-          /* If it's from a static pattern rule, convert the patterns into
-             "$*" so they'll expand properly.  */
-          if (d->staticpattern)
-            {
-              char *o;
-              char *buffer = variable_expand ("");
 
-              o = subst_expand (buffer, d->name, "%", "$*", 1, 2, 0);
+      /* If it's from a static pattern rule, convert the patterns into
+         "$*" so they'll expand properly.  */
+      if (d->staticpattern)
+        {
+          char *o;
+          d->name = o = variable_expand ("");
+          o = subst_expand (o, name, "%", "$*", 1, 2, 0);
+          *o = '\0';
+          free (name);
+          d->name = name = xstrdup (d->name);
+          d->staticpattern = 0;
+        }
 
-              d->name = strcache_add_len (variable_buffer,
-                                          o - variable_buffer);
-              d->staticpattern = 0; /* Clear staticpattern so that we don't
-                                       re-expand %s below. */
-            }
+      /* We're going to do second expansion so initialize file variables for
+         the file. Since the stem for static pattern rules comes from
+         individual dep lines, we will temporarily set f->stem to d->stem.  */
+      if (!initialized)
+        {
+          initialize_file_variables (f, 0);
+          initialized = 1;
+        }
 
-          /* We are going to do second expansion so initialize file variables
-             for the file. Since the stem for static pattern rules comes from
-             individual dep lines, we will temporarily set f->stem to d->stem.
-          */
-          if (!initialized)
-            {
-              initialize_file_variables (f, 0);
-              initialized = 1;
-            }
+      if (d->stem != 0)
+        f->stem = d->stem;
 
-          if (d->stem != 0)
-            f->stem = d->stem;
+      set_file_variables (f);
 
-          set_file_variables (f);
+      p = variable_expand_for_file (d->name, f);
 
-          p = variable_expand_for_file (d->name, f);
+      if (d->stem != 0)
+        f->stem = file_stem;
 
-          if (d->stem != 0)
-            f->stem = file_stem;
-        }
+      /* At this point we don't need the name anymore: free it.  */
+      free (name);
 
-      /* Parse the prerequisites.  */
-      new = parse_prereqs (p);
+      /* Parse the prerequisites and enter them into the file database.  */
+      new = enter_prereqs (split_prereqs (p), d->stem);
 
-      /* If this dep list was from a static pattern rule, expand the %s.  We
-         use patsubst_expand to translate the prerequisites' patterns into
-         plain prerequisite names.  */
-      if (new && d->staticpattern)
+      /* If there were no prereqs here (blank!) then throw this one out.  */
+      if (new == 0)
         {
-          const char *pattern = "%";
-          char *buffer = variable_expand ("");
-          struct dep *dp = new, *dl = 0;
-
-          while (dp != 0)
-            {
-              char *percent;
-              int nl = strlen (dp->name) + 1;
-              char *nm = alloca (nl);
-              memcpy (nm, dp->name, nl);
-              percent = find_percent (nm);
-              if (percent)
-                {
-                  char *o;
-
-                  /* We have to handle empty stems specially, because that
-                     would be equivalent to $(patsubst %,dp->name,) which
-                     will always be empty.  */
-                  if (d->stem[0] == '\0')
-                    {
-                      memmove (percent, percent+1, strlen (percent));
-                      o = variable_buffer_output (buffer, nm, strlen (nm) + 1);
-                    }
-                  else
-                    o = patsubst_expand_pat (buffer, d->stem, pattern, nm,
-                                             pattern+1, percent+1);
-
-                  /* If the name expanded to the empty string, ignore it.  */
-                  if (buffer[0] == '\0')
-                    {
-                      struct dep *df = dp;
-                      if (dp == new)
-                        dp = new = new->next;
-                      else
-                        dp = dl->next = dp->next;
-                      free_dep (df);
-                      continue;
-                    }
-
-                  /* Save the name.  */
-                  dp->name = strcache_add_len (buffer, o - buffer);
-                }
-              dl = dp;
-              dp = dp->next;
-            }
+          *dp = d->next;
+          free_dep (d);
+          d = *dp;
+          continue;
         }
 
-      /* Enter them as files. */
-      for (d1 = new; d1 != 0; d1 = d1->next)
-        {
-          d1->file = lookup_file (d1->name);
-          if (d1->file == 0)
-            d1->file = enter_file (d1->name);
-          d1->name = 0;
-          d1->staticpattern = 0;
-          d1->need_2nd_expansion = 0;
-        }
-
-      /* Add newly parsed deps to f->deps. If this is the last dependency
-         line and this target has commands then put it in front so the
-         last dependency line (the one with commands) ends up being the
-         first. This is important because people expect $< to hold first
-         prerequisite from the rule with commands. If it is not the last
-         dependency line or the rule does not have commands then link it
-         at the end so it appears in makefile order.  */
-
-      if (new != 0)
-        {
-          if (d->next == 0 && last_dep_has_cmds)
-            {
-              struct dep **d_ptr;
-              for (d_ptr = &new; *d_ptr; d_ptr = &(*d_ptr)->next)
-                ;
-
-              *d_ptr = f->deps;
-              f->deps = new;
-            }
-          else
-            {
-              struct dep **d_ptr;
-              for (d_ptr = &f->deps; *d_ptr; d_ptr = &(*d_ptr)->next)
-                ;
-
-              *d_ptr = new;
-            }
-        }
+      /* Add newly parsed prerequisites.  */
+      next = d->next;
+      *dp = new;
+      for (dp = &new->next, d = new->next; d != 0; dp = &d->next, d = d->next)
+        ;
+      *dp = next;
+      d = *dp;
     }
+}
+
+/* Reset the updating flag.  */
 
-  free_dep_chain (old);
+static void
+reset_updating (const void *item)
+{
+  struct file *f = (struct file *) item;
+  f->updating = 0;
 }
 
 /* For each dependency of each file, make the `struct dep' point
@@ -632,30 +640,44 @@ snap_deps (void)
   struct file *f;
   struct file *f2;
   struct dep *d;
-  struct file **file_slot_0;
-  struct file **file_slot;
-  struct file **file_end;
 
   /* Remember that we've done this.  Once we start snapping deps we can no
      longer define new targets.  */
   snapped_deps = 1;
 
-  /* Perform second expansion and enter each dependency name as a file. */
+  /* Perform second expansion and enter each dependency name as a file.  We
+     must use hash_dump() here because within these loops we likely add new
+     files to the table, possibly causing an in-situ table expansion.
 
-  /* Expand .SUFFIXES first; it's dependencies are used for $$* calculation. */
-  for (f = lookup_file (".SUFFIXES"); f != 0; f = f->prev)
-    expand_deps (f);
+     We only need to do this if second_expansion has been defined; if it
+     hasn't then all deps were expanded as the makefile was read in.  If we
+     ever change make to be able to unset .SECONDARY_EXPANSION this will have
+     to change.  */
 
-  /* For every target that's not .SUFFIXES, expand its dependencies.
-     We must use hash_dump (), because within this loop we might add new files
-     to the table, possibly causing an in-situ table expansion.  */
-  file_slot_0 = (struct file **) hash_dump (&files, 0, 0);
-  file_end = file_slot_0 + files.ht_fill;
-  for (file_slot = file_slot_0; file_slot < file_end; file_slot++)
-    for (f = *file_slot; f != 0; f = f->prev)
-      if (strcmp (f->name, ".SUFFIXES") != 0)
+  if (second_expansion)
+    {
+      struct file **file_slot_0 = (struct file **) hash_dump (&files, 0, 0);
+      struct file **file_end = file_slot_0 + files.ht_fill;
+      struct file **file_slot;
+      const char *suffixes;
+
+      /* Expand .SUFFIXES: its prerequisites are used for $$* calc.  */
+      f = lookup_file (".SUFFIXES");
+      suffixes = f ? f->name : 0;
+      for (; f != 0; f = f->prev)
         expand_deps (f);
-  free (file_slot_0);
+
+      /* For every target that's not .SUFFIXES, expand its prerequisites.  */
+
+      for (file_slot = file_slot_0; file_slot < file_end; file_slot++)
+        for (f = *file_slot; f != 0; f = f->prev)
+          if (f->name != suffixes)
+            expand_deps (f);
+      free (file_slot_0);
+    }
+  else
+    /* We're not doing second expansion, so reset updating.  */
+    hash_map (&files, reset_updating);
 
   /* Now manage all the special targets.  */
 
@@ -859,35 +881,40 @@ file_timestamp_sprintf (char *p, FILE_TIMESTAMP ts)
 
 /* Print the data base of files.  */
 
-static void
-print_file (const void *item)
+void
+print_prereqs (const struct dep *deps)
 {
-  const struct file *f = item;
-  struct dep *d;
-  struct dep *ood = 0;
-
-  putchar ('\n');
-  if (!f->is_target)
-    puts (_("# Not a target:"));
-  printf ("%s:%s", f->name, f->double_colon ? ":" : "");
+  const struct dep *ood = 0;
 
   /* Print all normal dependencies; note any order-only deps.  */
-  for (d = f->deps; d != 0; d = d->next)
-    if (! d->ignore_mtime)
-      printf (" %s", dep_name (d));
+  for (; deps != 0; deps = deps->next)
+    if (! deps->ignore_mtime)
+      printf (" %s", dep_name (deps));
     else if (! ood)
-      ood = d;
+      ood = deps;
 
   /* Print order-only deps, if we have any.  */
   if (ood)
     {
       printf (" | %s", dep_name (ood));
-      for (d = ood->next; d != 0; d = d->next)
-        if (d->ignore_mtime)
-          printf (" %s", dep_name (d));
+      for (ood = ood->next; ood != 0; ood = ood->next)
+        if (ood->ignore_mtime)
+          printf (" %s", dep_name (ood));
     }
 
   putchar ('\n');
+}
+
+static void
+print_file (const void *item)
+{
+  const struct file *f = item;
+
+  putchar ('\n');
+  if (!f->is_target)
+    puts (_("# Not a target:"));
+  printf ("%s:%s", f->name, f->double_colon ? ":" : "");
+  print_prereqs (f->deps);
 
   if (f->precious)
     puts (_("#  Precious file (prerequisite of .PRECIOUS)."));
@@ -906,6 +933,7 @@ print_file (const void *item)
     puts (_("#  File is an intermediate prerequisite."));
   if (f->also_make != 0)
     {
+      const struct dep *d;
       fputs (_("#  Also makes:"), stdout);
       for (d = f->also_make; d != 0; d = d->next)
 	printf (" %s", dep_name (d));
@@ -989,7 +1017,7 @@ print_file_data_base (void)
 #define VERIFY_CACHED(_p,_n) \
     do{\
         if (_p->_n && _p->_n[0] && !strcache_iscached (_p->_n)) \
-          printf ("%s: Field %s not cached: %s\n", _p->name, # _n, _p->_n); \
+          error (NULL, "%s: Field '%s' not cached: %s\n", _p->name, # _n, _p->_n); \
     }while(0)
 
 static void
@@ -1006,7 +1034,8 @@ verify_file (const void *item)
   /* Check the deps.  */
   for (d = f->deps; d != 0; d = d->next)
     {
-      VERIFY_CACHED (d, name);
+      if (! d->need_2nd_expansion)
+        VERIFY_CACHED (d, name);
       VERIFY_CACHED (d, stem);
     }
 }