From 0806a403d65ef6a7f16e2c17aa8286100ebad5d7 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 15 Feb 2006 23:54:42 +0000 Subject: [PATCH] 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. --- ChangeLog | 21 +++++ expand.c | 40 ++++---- function.c | 19 ++-- make.h | 1 + tests/ChangeLog | 12 +++ tests/scripts/functions/error | 22 +++-- tests/scripts/functions/foreach | 21 +++++ tests/scripts/functions/warning | 2 + tests/scripts/functions/word | 91 +++++++++++++------ .../scripts/options/warn-undefined-variables | 25 +++++ tests/scripts/variables/negative | 46 ++++++++++ 11 files changed, 242 insertions(+), 58 deletions(-) create mode 100644 tests/scripts/options/warn-undefined-variables create mode 100644 tests/scripts/variables/negative diff --git a/ChangeLog b/ChangeLog index 24732d27..e86bc3e6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2006-02-15 Paul D. Smith + + 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 * read.c (eval): Even if the included filenames expands to the diff --git a/expand.c b/expand.c index 82e12f7a..d25afef7 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 51c7e0c7..07cbbffd 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 d9f498fa..994f4f22 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 31dcbb29..c1342cb0 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,15 @@ +2006-02-15 Paul D. Smith + + * 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 * run_make_tests.pl (set_more_defaults): Update valgrind support diff --git a/tests/scripts/functions/error b/tests/scripts/functions/error index ca9b4e44..0d61177f 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 1fde12ef..904c1607 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 ac0ad643..cd452d41 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 398cd65e..34527ea7 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)) +wordlist-e3: ; @echo $(wordlist 1, 12a ,$(FOO))', + 'word-e1', + "#MAKEFILE#:3: *** non-numeric first argument to `word' function: ''. Stop.", + 512); -EOF +run_make_test(undef, + 'word-e2', + "#MAKEFILE#:4: *** non-numeric first argument to `word' function: 'abc '. Stop.", + 512); -close(MAKEFILE); +run_make_test(undef, + 'word-e3', + "#MAKEFILE#:5: *** non-numeric first argument to `word' function: '1a'. Stop.", + 512); -&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_test(undef, + 'wordlist-e1', + "#MAKEFILE#:7: *** non-numeric first argument to `wordlist' function: ''. Stop.", + 512); -&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_test(undef, + 'wordlist-e2', + "#MAKEFILE#:8: *** non-numeric first argument to `wordlist' function: 'abc '. Stop.", + 512); -&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_test(undef, + 'wordlist-e3', + "#MAKEFILE#:9: *** non-numeric second argument to `wordlist' function: ' 12a '. Stop.", + 512); -&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)); +# Test error conditions again, but this time in a variable reference -&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_test('FOO = foo bar biz baz -&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)); +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 00000000..34bfaea4 --- /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 00000000..16a72b89 --- /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;