Fix Savannah bug #106: keep separate track of which variable we are

expanding, and use that info when generating error messages instead of
the file info, where appropriate.

11 files changed, 250 insertions, 66 deletions

diff --git a/ChangeLog b/ChangeLog
index 24732d2..e86bc3e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2006-02-15  Paul D. Smith  <psmith@gnu.org>
+
+	Fix for Savannah bug #106.
+
+	* expand.c (expanding_var): Keep track of which variable we're
+	expanding.  If no variable is being expanded, it's the same as
+	reading_file.
+	* make.h (expanding_var): Declare it.
+	* expand.c (recursively_expand_for_file): Set expanding_var to the
+	current variable we're expanding, unless there's no file info in
+	it (could happen if it comes from the command line or a default
+	variable).  Restore it before we exit.
+	* expand.c (variable_expand_string): Use the expanding_var file
+	info instead of the reading_file info.
+	* function.c (check_numeric): Ditto.
+	(func_word): Ditto.
+	(func_wordlist): Ditto.
+	(func_error): Ditto.
+	(expand_builtin_function): Ditto.
+	(handle_function): Ditto.
+
 2006-02-14  Paul D. Smith  <psmith@gnu.org>
 
 	* read.c (eval): Even if the included filenames expands to the
diff --git a/expand.c b/expand.c
index 82e12f7..d25afef 100644
--- a/expand.c
+++ b/expand.c
@@ -26,6 +26,10 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.  */
 #include "variable.h"
 #include "rule.h"
 
+/* Initially, any errors reported when expanding strings will be reported
+   against the file where the error appears.  */
+const struct floc **expanding_var = &reading_file;
+
 /* The next two describe the variable output buffer.
    This buffer is used to hold the variable-expansion of a line of the
    makefile.  It is made bigger with realloc whenever it is too small.
@@ -95,14 +99,25 @@ char *
 recursively_expand_for_file (struct variable *v, struct file *file)
 {
   char *value;
-  struct variable_set_list *save = 0;
+  const struct floc *this_var;
+  const struct floc **saved_varp;
+  struct variable_set_list *save;
   int set_reading = 0;
 
+  /* Don't install a new location if this location is empty.
+     This can happen for command-line variables, builtin variables, etc.  */
+  saved_varp = expanding_var;
+  if (v->fileinfo.filenm)
+    {
+      this_var = &v->fileinfo;
+      expanding_var = &this_var;
+    }
+
   if (v->expanding)
     {
       if (!v->exp_count)
         /* Expanding V causes infinite recursion.  Lose.  */
-        fatal (reading_file,
+        fatal (this_var,
                _("Recursive variable `%s' references itself (eventually)"),
                v->name);
       --v->exp_count;
@@ -118,7 +133,7 @@ recursively_expand_for_file (struct variable *v, struct file *file)
   if (!reading_file)
     {
       set_reading = 1;
-      reading_file = &v->fileinfo;
+      reading_file = this_var;
     }
 
   v->expanding = 1;
@@ -130,9 +145,12 @@ recursively_expand_for_file (struct variable *v, struct file *file)
 
   if (set_reading)
     reading_file = 0;
+
   if (file)
     current_variable_set_list = save;
 
+  expanding_var = saved_varp;
+
   return value;
 }
 
@@ -245,7 +263,7 @@ variable_expand_string (char *line, char *string, long length)
 	    end = strchr (beg, closeparen);
 	    if (end == 0)
               /* Unterminated variable reference.  */
-              fatal (reading_file, _("unterminated variable reference"));
+              fatal (*expanding_var, _("unterminated variable reference"));
 	    p1 = lindex (beg, end, '$');
 	    if (p1 != 0)
 	      {
@@ -371,19 +389,7 @@ variable_expand_string (char *line, char *string, long length)
 
 	  /* A $ followed by a random char is a variable reference:
 	     $a is equivalent to $(a).  */
-	  {
-	    /* We could do the expanding here, but this way
-	       avoids code repetition at a small performance cost.  */
-	    char name[5];
-	    name[0] = '$';
-	    name[1] = '(';
-	    name[2] = *p;
-	    name[3] = ')';
-	    name[4] = '\0';
-	    p1 = allocated_variable_expand (name);
-	    o = variable_buffer_output (o, p1, strlen (p1));
-	    free (p1);
-	  }
+          o = reference_variable (o, p, 1);
 
 	  break;
 	}
diff --git a/function.c b/function.c
index 51c7e0c..07cbbff 100644
--- a/function.c
+++ b/function.c
@@ -743,7 +743,7 @@ check_numeric (const char *s, const char *message)
       break;
 
   if (s <= end || end - beg < 0)
-    fatal (reading_file, "%s: '%s'", message, beg);
+    fatal (*expanding_var, "%s: '%s'", message, beg);
 }
 
 
@@ -760,7 +760,8 @@ func_word (char *o, char **argv, const char *funcname UNUSED)
   i =  atoi (argv[0]);
 
   if (i == 0)
-    fatal (reading_file, _("first argument to `word' function must be greater than 0"));
+    fatal (*expanding_var,
+           _("first argument to `word' function must be greater than 0"));
 
 
   end_p = argv[1];
@@ -787,7 +788,7 @@ func_wordlist (char *o, char **argv, const char *funcname UNUSED)
 
   start = atoi (argv[0]);
   if (start < 1)
-    fatal (reading_file,
+    fatal (*expanding_var,
            "invalid first argument to `wordlist' function: `%d'", start);
 
   count = atoi (argv[1]) - start + 1;
@@ -1115,7 +1116,7 @@ func_error (char *o, char **argv, const char *funcname)
       break;
 
     default:
-      fatal (reading_file, "Internal error: func_error: '%s'", funcname);
+      fatal (*expanding_var, "Internal error: func_error: '%s'", funcname);
   }
 
   /* The warning function expands to the empty string.  */
@@ -2100,8 +2101,8 @@ expand_builtin_function (char *o, int argc, char **argv,
                          const struct function_table_entry *entry_p)
 {
   if (argc < (int)entry_p->minimum_args)
-    fatal (reading_file,
-           _("Insufficient number of arguments (%d) to function `%s'"),
+    fatal (*expanding_var,
+           _("insufficient number of arguments (%d) to function `%s'"),
            argc, entry_p->name);
 
   /* I suppose technically some function could do something with no
@@ -2112,8 +2113,8 @@ expand_builtin_function (char *o, int argc, char **argv,
     return o;
 
   if (!entry_p->func_ptr)
-    fatal (reading_file, _("Unimplemented on this platform: function `%s'"),
-           entry_p->name);
+    fatal (*expanding_var,
+           _("unimplemented on this platform: function `%s'"), entry_p->name);
 
   return entry_p->func_ptr (o, argv, entry_p->name);
 }
@@ -2162,7 +2163,7 @@ handle_function (char **op, char **stringp)
       break;
 
   if (count >= 0)
-    fatal (reading_file,
+    fatal (*expanding_var,
 	   _("unterminated call to function `%s': missing `%c'"),
 	   entry_p->name, closeparen);
 
diff --git a/make.h b/make.h
index d9f498f..994f4f2 100644
--- a/make.h
+++ b/make.h
@@ -497,6 +497,7 @@ extern char *getwd ();
 #endif
 
 extern const struct floc *reading_file;
+extern const struct floc **expanding_var;
 
 extern char **environ;
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 31dcbb2..c1342cb 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,15 @@
+2006-02-15  Paul D. Smith  <psmith@gnu.org>
+
+	* scripts/functions/error: Make sure filename/lineno information
+	is related to where the error is expanded, not where it's set.
+	* scripts/functions/warning: Ditto.
+	* scripts/functions/foreach: Check for different error conditions.
+	* scripts/functions/word: Ditto.
+	* scripts/variables/negative: Test some variable reference failure
+	conditions.
+	* scripts/options/warn-undefined-variables: Test the
+	--warn-undefined-variables flag.
+
 2006-02-09  Paul D. Smith  <psmith@gnu.org>
 
 	* run_make_tests.pl (set_more_defaults): Update valgrind support
diff --git a/tests/scripts/functions/error b/tests/scripts/functions/error
index ca9b4e4..0d61177 100644
--- a/tests/scripts/functions/error
+++ b/tests/scripts/functions/error
@@ -1,3 +1,5 @@
+#                                                                    -*-Perl-*-
+
 $description = "\
 The following test creates a makefile to test the error function.";
 
@@ -5,7 +7,8 @@ $details = "";
 
 open(MAKEFILE,"> $makefile");
 
-print MAKEFILE <<'EOF';
+print MAKEFILE 'err = $(error Error found!)
+
 ifdef ERROR1
 $(error error is $(ERROR1))
 endif
@@ -25,32 +28,39 @@ endif
 
 some: ; @echo Some stuff
 
-EOF
+testvar: ; @: $(err)
+';
 
 close(MAKEFILE);
 
 # Test #1
 
 &run_make_with_options($makefile, "ERROR1=yes", &get_logfile, 512);
-$answer = "$makefile:2: *** error is yes.  Stop.\n";
+$answer = "$makefile:4: *** error is yes.  Stop.\n";
 &compare_output($answer,&get_logfile(1));
 
 # Test #2
 
 &run_make_with_options($makefile, "ERROR2=no", &get_logfile, 512);
-$answer = "$makefile:6: *** error is no.  Stop.\n";
+$answer = "$makefile:8: *** error is no.  Stop.\n";
 &compare_output($answer,&get_logfile(1));
 
 # Test #3
 
 &run_make_with_options($makefile, "ERROR3=maybe", &get_logfile, 512);
-$answer = "Some stuff\n$makefile:10: *** error is maybe.  Stop.\n";
+$answer = "Some stuff\n$makefile:12: *** error is maybe.  Stop.\n";
 &compare_output($answer,&get_logfile(1));
 
 # Test #4
 
 &run_make_with_options($makefile, "ERROR4=definitely", &get_logfile, 512);
-$answer = "Some stuff\n$makefile:14: *** error is definitely.  Stop.\n";
+$answer = "Some stuff\n$makefile:16: *** error is definitely.  Stop.\n";
+&compare_output($answer,&get_logfile(1));
+
+# Test #5
+
+&run_make_with_options($makefile, "testvar", &get_logfile, 512);
+$answer = "$makefile:22: *** Error found!.  Stop.\n";
 &compare_output($answer,&get_logfile(1));
 
 # This tells the test driver that the perl test script executed properly.
diff --git a/tests/scripts/functions/foreach b/tests/scripts/functions/foreach
index 1fde12e..904c160 100644
--- a/tests/scripts/functions/foreach
+++ b/tests/scripts/functions/foreach
@@ -57,4 +57,25 @@ $(foreach x,FOREACH,$(eval $(value mktarget)))',
               '',
               'FOREACH');
 
+
+# TEST 2: Check some error conditions.
+
+run_make_test('
+x = $(foreach )
+y = $x
+
+all: ; @echo $y',
+              '',
+              "#MAKEFILE#:2: *** insufficient number of arguments (1) to function `foreach'.  Stop.",
+              512);
+
+run_make_test('
+x = $(foreach )
+y := $x
+
+all: ; @echo $y',
+              '',
+              "#MAKEFILE#:2: *** insufficient number of arguments (1) to function `foreach'.  Stop.",
+              512);
+
 1;
diff --git a/tests/scripts/functions/warning b/tests/scripts/functions/warning
index ac0ad64..cd452d4 100644
--- a/tests/scripts/functions/warning
+++ b/tests/scripts/functions/warning
@@ -1,3 +1,5 @@
+#                                                                    -*-Perl-*-
+
 $description = "\
 The following test creates a makefile to test the warning function.";
 
diff --git a/tests/scripts/functions/word b/tests/scripts/functions/word
index 398cd65..34527ea 100644
--- a/tests/scripts/functions/word
+++ b/tests/scripts/functions/word
@@ -46,11 +46,7 @@ $answer = "6\n"
 
 # Test error conditions
 
-$makefile2 = &get_tmpfile;
-
-open(MAKEFILE, "> $makefile2");
-print MAKEFILE <<'EOF';
-FOO = foo bar biz baz
+run_make_test('FOO = foo bar biz baz
 
 word-e1: ; @echo $(word ,$(FOO))
 word-e2: ; @echo $(word abc ,$(FOO))
@@ -58,35 +54,78 @@ word-e3: ; @echo $(word 1a,$(FOO))
 
 wordlist-e1: ; @echo $(wordlist ,,$(FOO))
 wordlist-e2: ; @echo $(wordlist abc ,,$(FOO))
-wordlist-e3: ; @echo $(wordlist 1, 12a ,$(FOO))
-
-EOF
-
-close(MAKEFILE);
-
-&run_make_with_options($makefile2, 'word-e1', &get_logfile, 512);
-$answer = "$makefile2:3: *** non-numeric first argument to `word' function: ''.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
-
-&run_make_with_options($makefile2, 'word-e2', &get_logfile, 512);
-$answer = "$makefile2:4: *** non-numeric first argument to `word' function: 'abc '.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
-
-&run_make_with_options($makefile2, 'word-e3', &get_logfile, 512);
-$answer = "$makefile2:5: *** non-numeric first argument to `word' function: '1a'.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
-
-&run_make_with_options($makefile2, 'wordlist-e1', &get_logfile, 512);
-$answer = "$makefile2:7: *** non-numeric first argument to `wordlist' function: ''.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
-
-&run_make_with_options($makefile2, 'wordlist-e2', &get_logfile, 512);
-$answer = "$makefile2:8: *** non-numeric first argument to `wordlist' function: 'abc '.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
-
-&run_make_with_options($makefile2, 'wordlist-e3', &get_logfile, 512);
-$answer = "$makefile2:9: *** non-numeric second argument to `wordlist' function: ' 12a '.  Stop.\n";
-&compare_output($answer, &get_logfile(1));
+wordlist-e3: ; @echo $(wordlist 1, 12a ,$(FOO))',
+              'word-e1',
+              "#MAKEFILE#:3: *** non-numeric first argument to `word' function: ''.  Stop.",
+              512);
+
+run_make_test(undef,
+              'word-e2',
+              "#MAKEFILE#:4: *** non-numeric first argument to `word' function: 'abc '.  Stop.",
+              512);
+
+run_make_test(undef,
+              'word-e3',
+              "#MAKEFILE#:5: *** non-numeric first argument to `word' function: '1a'.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e1',
+              "#MAKEFILE#:7: *** non-numeric first argument to `wordlist' function: ''.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e2',
+              "#MAKEFILE#:8: *** non-numeric first argument to `wordlist' function: 'abc '.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e3',
+              "#MAKEFILE#:9: *** non-numeric second argument to `wordlist' function: ' 12a '.  Stop.",
+              512);
+
+# Test error conditions again, but this time in a variable reference
+
+run_make_test('FOO = foo bar biz baz
+
+W = $(word $x,$(FOO))
+WL = $(wordlist $s,$e,$(FOO))
+
+word-e: ; @echo $(W)
+wordlist-e: ; @echo $(WL)',
+              'word-e x=',
+              "#MAKEFILE#:3: *** non-numeric first argument to `word' function: ''.  Stop.",
+              512);
+
+run_make_test(undef,
+              'word-e x=abc',
+              "#MAKEFILE#:3: *** non-numeric first argument to `word' function: 'abc'.  Stop.",
+              512);
+
+run_make_test(undef,
+              'word-e x=0',
+              "#MAKEFILE#:3: *** first argument to `word' function must be greater than 0.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e s= e=',
+              "#MAKEFILE#:4: *** non-numeric first argument to `wordlist' function: ''.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e s=abc e=',
+              "#MAKEFILE#:4: *** non-numeric first argument to `wordlist' function: 'abc'.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e s=4 e=12a',
+              "#MAKEFILE#:4: *** non-numeric second argument to `wordlist' function: '12a'.  Stop.",
+              512);
+
+run_make_test(undef,
+              'wordlist-e s=0 e=12',
+              "#MAKEFILE#:4: *** invalid first argument to `wordlist' function: `0'.  Stop.",
+              512);
 
 
 # TEST #8 -- test $(firstword )
diff --git a/tests/scripts/options/warn-undefined-variables b/tests/scripts/options/warn-undefined-variables
new file mode 100644
index 0000000..34bfaea
--- /dev/null
+++ b/tests/scripts/options/warn-undefined-variables
@@ -0,0 +1,25 @@
+#                                                                    -*-perl-*-
+
+$description = "Test the --warn-undefined-variables option.";
+
+$details = "Verify that warnings are printed for referencing undefined variables.";
+
+# Without --warn-undefined-variables, nothing should happen
+run_make_test('
+EMPTY =
+EREF = $(EMPTY)
+UREF = $(UNDEFINED)
+
+SEREF := $(EREF)
+SUREF := $(UREF)
+
+all: ; @echo ref $(EREF) $(UREF)',
+              '', 'ref');
+
+# With --warn-undefined-variables, it should warn me
+run_make_test(undef, '--warn-undefined-variables',
+              "#MAKEFILE#:7: warning: undefined variable `UNDEFINED'
+#MAKEFILE#:9: warning: undefined variable `UNDEFINED'
+ref");
+
+1;
diff --git a/tests/scripts/variables/negative b/tests/scripts/variables/negative
new file mode 100644
index 0000000..16a72b8
--- /dev/null
+++ b/tests/scripts/variables/negative
@@ -0,0 +1,46 @@
+#                                                                    -*-perl-*-
+
+$description = "Run some negative tests (things that should fail).";
+
+# TEST #0
+# Check that non-terminated variable references are detected (and
+# reported using the best filename/lineno info
+run_make_test('
+foo = bar
+x = $(foo
+y = $x
+
+all: ; @echo $y
+',
+              '', '#MAKEFILE#:3: *** unterminated variable reference.  Stop.',
+              512);
+
+# TEST #1
+# Bogus variable value passed on the command line.
+run_make_test(undef,
+              'x=\$\(other',
+              '#MAKEFILE#:4: *** unterminated variable reference.  Stop.',
+              512);
+
+# TEST #2
+# Again, but this time while reading the makefile.
+run_make_test('
+foo = bar
+x = $(foo
+y = $x
+
+z := $y
+
+all: ; @echo $y
+',
+              '', '#MAKEFILE#:3: *** unterminated variable reference.  Stop.',
+              512);
+
+# TEST #3
+# Bogus variable value passed on the command line.
+run_make_test(undef,
+              'x=\$\(other',
+              '#MAKEFILE#:4: *** unterminated variable reference.  Stop.',
+              512);
+
+1;