[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.
diff --git a/tests/scripts/variables/special b/tests/scripts/variables/special
index a5ab93a..2c0b42c 100644
--- a/tests/scripts/variables/special
+++ b/tests/scripts/variables/special
@@ -14,14 +14,22 @@
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 @@
# 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 @@
: 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 @@
.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 @@
: test-two \
test-three \
test-four \
- test-five \\\\
+ test-five \\\\
: test-six
: foo-one foo-two
: foo-three
@@ -108,7 +116,7 @@
: 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 4b50503..35ba4f2 100644
--- a/variable.c
+++ b/variable.c
@@ -29,6 +29,9 @@
#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 @@
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 @@
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 @@
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 @@
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 @@
}
*(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 @@
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 @@
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 */
