From 9ae02b79167e66311d23979cf1433d76054b86f8 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 13 Mar 2016 18:13:00 -0400 Subject: [PATCH] [SV 45728] Detect changes in .VARIABLES more accurately. For performance, we only recompute .VARIABLES when (a) it's expanded and (b) when its value will change from a previous expansion. To determine (b) we were checking the number of entries in the hash table which used to work until we started undefining entries: now if you undefine and redefine the same number of entries in between expanding .VARIABLES, it doesn't detect any change. Instead, keep an increasing change number. * variables.c: Add variable_changenum. (define_variable_in_set, merge_variable_sets): Increment variable_changenum if adding a new variable to the global set. (undefine_variable_in_set): Increment variable_changenum if undefining a variable from the global set. (lookup_special_var): Test variable_changenum not the hash table. * tests/scripts/variables/special: Test undefining variables. --- tests/scripts/variables/special | 50 +++++++++++++++++++-------------- variable.c | 31 ++++++++++++-------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/tests/scripts/variables/special b/tests/scripts/variables/special index a5ab93aa..2c0b42ca 100644 --- a/tests/scripts/variables/special +++ b/tests/scripts/variables/special @@ -14,14 +14,22 @@ X2 := $(sort $(filter FOO BAR,$(.VARIABLES))) BAR := bar -all: - @echo X1 = $(X1) - @echo X2 = $(X2) - @echo LAST = $(sort $(filter FOO BAR,$(.VARIABLES))) +all: ; @echo X1 = $(X1); echo X2 = $(X2); echo LAST = $(sort $(filter FOO BAR,$(.VARIABLES))) ', '', "X1 =\nX2 = FOO\nLAST = BAR FOO\n"); +# SV 45728: Test that undefining a variable is reflected properly +&run_make_test(' +FOO := foo +BAR := bar +$(info one: $(sort $(filter FOO BAR BAZ,$(.VARIABLES)))) +undefine BAR +BAZ := baz +$(info two: $(sort $(filter FOO BAR BAZ,$(.VARIABLES)))) +all:;@: +', + '', "one: BAR FOO\ntwo: BAZ FOO\n"); # $makefile2 = &get_tmpfile; # open(MAKEFILE, "> $makefile2"); @@ -31,9 +39,9 @@ all: # X1 := $(sort $(.TARGETS)) # all: foo -# @echo X1 = $(X1) -# @echo X2 = $(X2) -# @echo LAST = $(sort $(.TARGETS)) +# @echo X1 = $(X1) +# @echo X2 = $(X2) +# @echo LAST = $(sort $(.TARGETS)) # X2 := $(sort $(.TARGETS)) @@ -56,16 +64,16 @@ define foo : foo-one\ foo-two : foo-three - : foo-four + : foo-four endef orig: ; : orig-one - : orig-two \ + : orig-two \ orig-three \ - orig-four \ - orig-five \\\\ - : orig-six - $(foo) + orig-four \ + orig-five \\\\ + : orig-six + $(foo) .RECIPEPREFIX = > test: ; : test-one @@ -78,19 +86,19 @@ test-three \ .RECIPEPREFIX = reset: ; : reset-one - : reset-two \ + : reset-two \ reset-three \ - reset-four \ - reset-five \\\\ - : reset-six - $(foo) + reset-four \ + reset-five \\\\ + : reset-six + $(foo) ', 'orig test reset', ': orig-one : orig-two \ orig-three \ orig-four \ - orig-five \\\\ + orig-five \\\\ : orig-six : foo-one foo-two : foo-three @@ -99,7 +107,7 @@ orig-four \ : test-two \ test-three \ test-four \ - test-five \\\\ + test-five \\\\ : test-six : foo-one foo-two : foo-three @@ -108,7 +116,7 @@ test-four \ : reset-two \ reset-three \ reset-four \ - reset-five \\\\ + reset-five \\\\ : reset-six : foo-one foo-two : foo-three diff --git a/variable.c b/variable.c index 4b505039..35ba4f2a 100644 --- a/variable.c +++ b/variable.c @@ -29,6 +29,9 @@ this program. If not, see . */ #endif #include "hash.h" +/* Incremented every time we add or remove a global variable. */ +static unsigned int variable_changenum; + /* Chain of all pattern-specific variables. */ static struct pattern_var *pattern_vars; @@ -268,6 +271,9 @@ define_variable_in_set (const char *name, unsigned int length, v->name = xstrndup (name, length); v->length = length; hash_insert_at (&set->table, v, var_slot); + if (set == &global_variable_set) + ++variable_changenum; + v->value = xstrdup (value); if (flocp != 0) v->fileinfo = *flocp; @@ -350,12 +356,14 @@ undefine_variable_in_set (const char *name, unsigned int length, before the switches were parsed, it wasn't affected by -e. */ v->origin = o_env_override; - /* If the definition is from a stronger source than this one, don't - undefine it. */ + /* Undefine only if this undefinition is from an equal or stronger + source than the variable definition. */ if ((int) origin >= (int) v->origin) { hash_delete_at (&set->table, var_slot); free_variable_name_and_value (v); + if (set == &global_variable_set) + ++variable_changenum; } } } @@ -373,7 +381,7 @@ undefine_variable_in_set (const char *name, unsigned int length, static struct variable * lookup_special_var (struct variable *var) { - static unsigned long last_var_count = 0; + static unsigned long last_changenum = 0; /* This one actually turns out to be very hard, due to the way the parser @@ -401,8 +409,7 @@ lookup_special_var (struct variable *var) else */ - if (streq (var->name, ".VARIABLES") - && global_variable_set.table.ht_fill != last_var_count) + if (variable_changenum != last_changenum && streq (var->name, ".VARIABLES")) { unsigned long max = EXPANSION_INCREMENT (strlen (var->value)); unsigned long len; @@ -438,11 +445,8 @@ lookup_special_var (struct variable *var) } *(p-1) = '\0'; - /* Remember how many variables are in our current count. Since we never - remove variables from the list, this is a reliable way to know whether - the list is up to date or needs to be recomputed. */ - - last_var_count = global_variable_set.table.ht_fill; + /* Remember the current variable change number. */ + last_changenum = variable_changenum; } return var; @@ -753,6 +757,8 @@ merge_variable_sets (struct variable_set *to_set, struct variable **from_var_slot = (struct variable **) from_set->table.ht_vec; struct variable **from_var_end = from_var_slot + from_set->table.ht_size; + int inc = to_set == &global_variable_set ? 1 : 0; + for ( ; from_var_slot < from_var_end; from_var_slot++) if (! HASH_VACANT (*from_var_slot)) { @@ -760,7 +766,10 @@ merge_variable_sets (struct variable_set *to_set, struct variable **to_var_slot = (struct variable **) hash_find_slot (&to_set->table, *from_var_slot); if (HASH_VACANT (*to_var_slot)) - hash_insert_at (&to_set->table, from_var, to_var_slot); + { + hash_insert_at (&to_set->table, from_var, to_var_slot); + variable_changenum += inc; + } else { /* GKM FIXME: delete in from_set->table */