Fix Savannah bug #11913: ensure that scopes such as foreach, etc. take

precedence over the global scope when they're used in a global context
(such as an eval).

6 files changed, 106 insertions, 41 deletions

diff --git a/ChangeLog b/ChangeLog
index 318cbd7..6482a38 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2005-06-09  Paul D. Smith  <psmith@gnu.org>
+
+        * variable.c (push_new_variable_scope): File variables point
+        directly to the global_setlist variable.  So, inserting a new
+        scope in front of that has no effect on those variables: they
+        don't go through current_variable_set_list.  If we're pushing a
+        scope and the current scope is global, push it "the other way" so
+        that the new setlist is in the global_setlist variable, and
+        next points to a new setlist with the global variable set.
+        (pop_variable_scope): Properly undo a push with the new
+        semantics.
+        Fixes Savannah bug #11913.
+
 2005-05-31  Boris Kolpackov  <boris@kolpackov.net>
 
 	* job.c (reap_children): Don't die of the command failed but
diff --git a/NEWS b/NEWS
index 7b863a5..b3a5634 100644
--- a/NEWS
+++ b/NEWS
@@ -36,7 +36,8 @@ Version 3.81beta3
 
 * The "else" conditional line can now be followed by any other legal
   conditional on the same line: this does not increase the depth of the
-  conditional nesting.
+  conditional nesting, so only one "endif" is required to close the
+  conditional.
 
 * All pattern-specific variables that match a given target are now used
   (previously only the first match was used).
diff --git a/tests/ChangeLog b/tests/ChangeLog
index b33269c..ffb3fe0 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2005-06-09  Paul D. Smith  <psmith@gnu.org>
+
+        * scripts/functions/foreach: Add a test for Savannah bug #11913.
+
 2005-05-31  Boris Kolpackov  <boris@kolpackov.net>
 
 	* scripts/features/include: Add a test for Savannah bug #13216.
diff --git a/tests/scripts/functions/foreach b/tests/scripts/functions/foreach
index b80751b..1fde12e 100644
--- a/tests/scripts/functions/foreach
+++ b/tests/scripts/functions/foreach
@@ -1,6 +1,6 @@
 #                                                                    -*-perl-*-
 
-# Updated 6.16.93  variable "MAKE" is default was environment override
+# Updated 16 June 1993 variable "MAKE" is default was environment override
 # For make 3.63 and above
 
 $description = "The following test creates a makefile to verify
@@ -14,40 +14,47 @@ form of the command is $(foreach var,$list,$text). Several
 types of foreach loops are tested\n";
 
 
-open(MAKEFILE,"> $makefile");
-
-# The Contents of the MAKEFILE ...
+# TEST 0
 
 # On WIN32 systems, the user's path is found in %Path% ($Path)
 #
 $pathvar = (($port_type eq 'Windows') ? "Path" : "PATH");
 
-print MAKEFILE <<EOF;
-foo = bletch null \@ garf
+run_make_test("
 null :=
 space = ' '
-auto_var = udef space CC null $pathvar MAKE foo CFLAGS WHITE \@ <
-av = \$(foreach var, \$(auto_var), \$(origin \$(var)) )
+auto_var = udef space CC null $pathvar".' MAKE foo CFLAGS WHITE @ <
+foo = bletch null @ garf
+av = $(foreach var, $(auto_var), $(origin $(var)) )
 override WHITE := BLACK
-for_var = \$(addsuffix .c,foo \$(null) \$(foo) \$(space) \$(av) )
-fe = \$(foreach var2, \$(for_var),\$(subst .c,.o, \$(var2) ) )
+for_var = $(addsuffix .c,foo $(null) $(foo) $(space) $(av) )
+fe = $(foreach var2, $(for_var),$(subst .c,.o, $(var2) ) )
 all: auto for2
-auto :
-\t\@echo \$(av)
-for2:
-\t\@echo \$(fe)
-EOF
+auto : ; @echo $(av)
+for2: ; @echo $(fe)',
+              '-e WHITE=WHITE CFLAGS=',
+              "undefined file default file environment default file command line override automatic automatic
+foo.o bletch.o null.o @.o garf.o .o    .o undefined.o file.o default.o file.o environment.o default.o file.o command.o line.o override.o automatic.o automatic.o");
+
+
+# TEST 1: Test that foreach variables take precedence over global
+# variables in a global scope (like inside an eval).  Tests bug #11913
+
+run_make_test('
+.PHONY: all target
+all: target
 
-close(MAKEFILE);
+x := BAD
 
-&run_make_with_options($makefile,
-                      "-e WHITE=WHITE CFLAGS=",
-                      &get_logfile);
+define mktarget
+target: x := $(x)
+target: ; @echo "$(x)"
+endef
 
-# Create the answer to what should be produced by this Makefile
-$answer = "undefined file default file environment default file command line override automatic automatic
-foo.o bletch.o null.o @.o garf.o .o    .o undefined.o file.o default.o file.o environment.o default.o file.o command.o line.o override.o automatic.o automatic.o\n";
+x := GLOBAL
 
-&compare_output($answer,&get_logfile(1));
+$(foreach x,FOREACH,$(eval $(value mktarget)))',
+              '',
+              'FOREACH');
 
 1;
diff --git a/variable.c b/variable.c
index 7e26646..2f1826f 100644
--- a/variable.c
+++ b/variable.c
@@ -19,6 +19,9 @@ the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
 Boston, MA 02111-1307, USA.  */
 
 #include "make.h"
+
+#include <assert.h>
+
 #include "dep.h"
 #include "filedef.h"
 #include "job.h"
@@ -546,21 +549,6 @@ free_variable_name_and_value (const void *item)
   free (v->value);
 }
 
-void
-pop_variable_scope (void)
-{
-  struct variable_set_list *setlist = current_variable_set_list;
-  struct variable_set *set = setlist->set;
-
-  current_variable_set_list = setlist->next;
-  free ((char *) setlist);
-
-  hash_map (&set->table, free_variable_name_and_value);
-  hash_free (&set->table, 1);
-
-  free ((char *) set);
-}
-
 struct variable_set_list *
 create_new_variable_set (void)
 {
@@ -579,12 +567,63 @@ create_new_variable_set (void)
   return setlist;
 }
 
-/* Create a new variable set and push it on the current setlist.  */
+/* Create a new variable set and push it on the current setlist.
+   If we're pushing a global scope (that is, the current scope is the global
+   scope) then we need to "push" it the other way: file variable sets point
+   directly to the global_setlist so we need to replace that with the new one.
+ */
 
 struct variable_set_list *
 push_new_variable_scope (void)
 {
-  return (current_variable_set_list = create_new_variable_set());
+  current_variable_set_list = create_new_variable_set();
+  if (current_variable_set_list->next == &global_setlist)
+    {
+      /* It was the global, so instead of new -> &global we want to replace
+         &global with the new one and have &global -> new, with current still
+         pointing to &global  */
+      struct variable_set *set = current_variable_set_list->set;
+      current_variable_set_list->set = global_setlist.set;
+      global_setlist.set = set;
+      current_variable_set_list->next = global_setlist.next;
+      global_setlist.next = current_variable_set_list;
+      current_variable_set_list = &global_setlist;
+    }
+  return (current_variable_set_list);
+}
+
+void
+pop_variable_scope (void)
+{
+  struct variable_set_list *setlist;
+  struct variable_set *set;
+
+  /* Can't call this if there's no scope to pop!  */
+  assert(current_variable_set_list->next != NULL);
+
+  if (current_variable_set_list != &global_setlist)
+    {
+      /* We're not pointing to the global setlist, so pop this one.  */
+      setlist = current_variable_set_list;
+      set = setlist->set;
+      current_variable_set_list = setlist->next;
+    }
+  else
+    {
+      /* This set is the one in the global_setlist, but there is another global
+         set beyond that.  We want to copy that set to global_setlist, then
+         delete what used to be in global_setlist.  */
+      setlist = global_setlist.next;
+      set = global_setlist.set;
+      global_setlist.set = setlist->set;
+      global_setlist.next = setlist->next;
+    }
+
+  /* Free the one we no longer need.  */
+  free ((char *) setlist);
+  hash_map (&set->table, free_variable_name_and_value);
+  hash_free (&set->table, 1);
+  free ((char *) set);
 }
 
 /* Merge FROM_SET into TO_SET, freeing unused storage in FROM_SET.  */
diff --git a/w32/.cvsignore b/w32/.cvsignore
index 6179e0d..efc8094 100644
--- a/w32/.cvsignore
+++ b/w32/.cvsignore
@@ -1 +1,2 @@
 Makefile Makefile.in
+.deps