From bd2d124f275cec912d33ec1463ba66e799518340 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 6 Oct 2009 12:36:29 +0000 Subject: [PATCH] Fix savannah bug 25780. Optimize things a bit. --- ChangeLog | 14 +++++ commands.c | 92 +++++++++++++++++++++++------- dep.h | 1 - implicit.c | 13 ++--- read.c | 64 --------------------- tests/ChangeLog | 5 ++ tests/scripts/features/se_explicit | 13 ++--- 7 files changed, 101 insertions(+), 101 deletions(-) diff --git a/ChangeLog b/ChangeLog index f68099f2..49375217 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2009-10-06 Boris Kolpackov + + * dep.h (uniquize_deps): Remove. + + * read.c (uniquize_deps): Merge into set_file_variables in + commands.c. + (dep_hash_1, dep_hash_2, dep_hash_cmp): Move to commands.c. + + * commands.c (set_file_variables): Avoid modifying the dep + chain to achieve uniqueness. Fixes savannah bug 25780. + + * implicit.c (pattern_search): Instead of re-setting all automatic + variables for each rule we try, just update $*. + 2009-10-06 Boris Kolpackov * variable.h (undefine_variable_in_set): New function declaration. diff --git a/commands.c b/commands.c index d9d7f539..8a7bb6c4 100644 --- a/commands.c +++ b/commands.c @@ -39,6 +39,36 @@ int remote_kill (int id, int sig); int getpid (); #endif + +static unsigned long +dep_hash_1 (const void *key) +{ + return_STRING_HASH_1 (dep_name ((struct dep const *) key)); +} + +static unsigned long +dep_hash_2 (const void *key) +{ + return_STRING_HASH_2 (dep_name ((struct dep const *) key)); +} + +static int +dep_hash_cmp (const void *x, const void *y) +{ + struct dep *dx = (struct dep *) x; + struct dep *dy = (struct dep *) y; + int cmp = strcmp (dep_name (dx), dep_name (dy)); + + /* If the names are the same but ignore_mtimes are not equal, one of these + is an order-only prerequisite and one isn't. That means that we should + remove the one that isn't and keep the one that is. */ + + if (!cmp && dx->ignore_mtime != dy->ignore_mtime) + dx->ignore_mtime = dy->ignore_mtime = 0; + + return cmp; +} + /* Set FILE's automatic variables up. */ void @@ -149,18 +179,34 @@ set_file_variables (struct file *file) char *bp; unsigned int len; + struct hash_table dep_hash; + void **slot; + /* Compute first the value for $+, which is supposed to contain duplicate dependencies as they were listed in the makefile. */ plus_len = 0; + bar_len = 0; for (d = file->deps; d != 0; d = d->next) - if (! d->ignore_mtime && ! d->need_2nd_expansion) - plus_len += strlen (dep_name (d)) + 1; + { + if (!d->need_2nd_expansion) + { + if (d->ignore_mtime) + bar_len += strlen (dep_name (d)) + 1; + else + plus_len += strlen (dep_name (d)) + 1; + } + } + + if (bar_len == 0) + bar_len++; + if (plus_len == 0) plus_len++; if (plus_len > plus_max) plus_value = xrealloc (plus_value, plus_max = plus_len); + cp = plus_value; qmark_len = plus_len + 1; /* Will be this or less. */ @@ -191,19 +237,6 @@ set_file_variables (struct file *file) cp[cp > plus_value ? -1 : 0] = '\0'; DEFINE_VARIABLE ("+", 1, plus_value); - /* Make sure that no dependencies are repeated. This does not - really matter for the purpose of updating targets, but it - might make some names be listed twice for $^ and $?. */ - - uniquize_deps (file->deps); - - bar_len = 0; - for (d = file->deps; d != 0; d = d->next) - if (d->ignore_mtime && ! d->need_2nd_expansion) - bar_len += strlen (dep_name (d)) + 1; - if (bar_len == 0) - bar_len++; - /* Compute the values for $^, $?, and $|. */ cp = caret_value = plus_value; /* Reuse the buffer; it's big enough. */ @@ -216,16 +249,33 @@ set_file_variables (struct file *file) bar_value = xrealloc (bar_value, bar_max = bar_len); bp = bar_value; + /* Make sure that no dependencies are repeated in $^, $?, and $|. It + would be natural to combine the next two loops but we can't do it + because of a situation where we have two dep entries, the first + is order-only and the second is normal (see dep_hash_cmp). */ + + hash_init (&dep_hash, 500, dep_hash_1, dep_hash_2, dep_hash_cmp); + for (d = file->deps; d != 0; d = d->next) { - const char *c; - if (d->need_2nd_expansion) continue; + slot = hash_find_slot (&dep_hash, d); + if (HASH_VACANT (*slot)) + hash_insert_at (&dep_hash, d, slot); + } + + for (d = file->deps; d != 0; d = d->next) + { + const char *c; + + if (d->need_2nd_expansion || hash_find_item (&dep_hash, d) != d) + continue; + c = dep_name (d); #ifndef NO_ARCHIVES - if (ar_name (c)) + if (ar_name (c)) { c = strchr (c, '(') + 1; len = strlen (c) - 1; @@ -236,12 +286,12 @@ set_file_variables (struct file *file) if (d->ignore_mtime) { - memcpy (bp, c, len); + memcpy (bp, c, len); bp += len; *bp++ = FILE_LIST_SEPARATOR; } else - { + { memcpy (cp, c, len); cp += len; *cp++ = FILE_LIST_SEPARATOR; @@ -254,6 +304,8 @@ set_file_variables (struct file *file) } } + hash_free (&dep_hash, 0); + /* Kill the last spaces and define the variables. */ cp[cp > caret_value ? -1 : 0] = '\0'; diff --git a/dep.h b/dep.h index 96964234..1f493132 100644 --- a/dep.h +++ b/dep.h @@ -90,4 +90,3 @@ void free_ns_chain (struct nameseq *n); struct dep *read_all_makefiles (const char **makefiles); int eval_buffer (char *buffer); int update_goal_chain (struct dep *goals); -void uniquize_deps (struct dep *); diff --git a/implicit.c b/implicit.c index 08e56bce..b4e843cb 100644 --- a/implicit.c +++ b/implicit.c @@ -612,20 +612,19 @@ pattern_search (struct file *file, int archive, add_dir = 1; } - /* Initialize file variables if we haven't already + /* Initialize and set file variables if we haven't already done so. */ if (!file_vars_initialized) { initialize_file_variables (file, 0); + set_file_variables (file); file_vars_initialized = 1; } - - /* Set file variables. Note that we cannot do it once at the - beginning of the function because the stem value changes - for each rule. */ - if (!file_variables_set) + /* Update the stem value in $* for this rule. */ + else if (!file_variables_set) { - set_file_variables (file); + define_variable_for_file ( + "*", 1, file->stem, o_automatic, 0, file); file_variables_set = 1; } diff --git a/read.c b/read.c index a86f7915..3a022d6c 100644 --- a/read.c +++ b/read.c @@ -1723,71 +1723,7 @@ conditional_line (char *line, int len, const struct floc *flocp) return 0; } -/* Remove duplicate dependencies in CHAIN. */ -static unsigned long -dep_hash_1 (const void *key) -{ - return_STRING_HASH_1 (dep_name ((struct dep const *) key)); -} - -static unsigned long -dep_hash_2 (const void *key) -{ - return_STRING_HASH_2 (dep_name ((struct dep const *) key)); -} - -static int -dep_hash_cmp (const void *x, const void *y) -{ - struct dep *dx = (struct dep *) x; - struct dep *dy = (struct dep *) y; - int cmp = strcmp (dep_name (dx), dep_name (dy)); - - /* If the names are the same but ignore_mtimes are not equal, one of these - is an order-only prerequisite and one isn't. That means that we should - remove the one that isn't and keep the one that is. */ - - if (!cmp && dx->ignore_mtime != dy->ignore_mtime) - dx->ignore_mtime = dy->ignore_mtime = 0; - - return cmp; -} - - -void -uniquize_deps (struct dep *chain) -{ - struct hash_table deps; - register struct dep **depp; - - hash_init (&deps, 500, dep_hash_1, dep_hash_2, dep_hash_cmp); - - /* Make sure that no dependencies are repeated. This does not - really matter for the purpose of updating targets, but it - might make some names be listed twice for $^ and $?. */ - - depp = &chain; - while (*depp) - { - struct dep *dep = *depp; - struct dep **dep_slot = (struct dep **) hash_find_slot (&deps, dep); - if (HASH_VACANT (*dep_slot)) - { - hash_insert_at (&deps, dep, dep_slot); - depp = &dep->next; - } - else - { - /* Don't bother freeing duplicates. - It's dangerous and little benefit accrues. */ - *depp = dep->next; - } - } - - hash_free (&deps, 0); -} - /* Record target-specific variable values for files FILENAMES. TWO_COLON is nonzero if a double colon was used. diff --git a/tests/ChangeLog b/tests/ChangeLog index 939a5f50..bfd56718 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2009-10-06 Boris Kolpackov + + * scripts/features/se_explicit: Enable the test for now fixed + savannah bug 25780. + 2009-10-06 Boris Kolpackov * scripts/variables/undefine: Tests for the new undefine feature. diff --git a/tests/scripts/features/se_explicit b/tests/scripts/features/se_explicit index 1b514749..79e0a36f 100644 --- a/tests/scripts/features/se_explicit +++ b/tests/scripts/features/se_explicit @@ -131,9 +131,8 @@ endef '', "#MAKE#: *** prerequisites cannot be defined in recipes. Stop.\n", 512); -if ($all_tests) { - # Automatic $$+ variable expansion issue. Savannah bug #25780 - run_make_test(q! +# Automatic $$+ variable expansion issue. Savannah bug #25780 +run_make_test(q! all : foo foo .SECONDEXPANSION: all : $$+ ; @echo '$+' @@ -141,11 +140,9 @@ foo : ; !, '', "foo foo foo foo\n"); -} -if ($all_tests) { - # Automatic $$+ variable expansion issue. Savannah bug #25780 - run_make_test(q! +# Automatic $$+ variable expansion issue. Savannah bug #25780 +run_make_test(q! all : bar bar bar : ; q%x : ; @@ -154,8 +151,6 @@ a%l: q1x $$+ q2x ; @echo '$+' !, '', "q1x bar bar q2x bar bar\n"); -} - # This tells the test driver that the perl test script executed properly. 1;