[src/trunk]: src/usr.bin/make make(1): fix unexpected behavior in ::= variabl...

To: source-changes-hg%NetBSD.org@localhost
Subject: [src/trunk]: src/usr.bin/make make(1): fix unexpected behavior in ::= variabl...
From: rillig <rillig%NetBSD.org@localhost>
Date: Wed, 23 Sep 2020 11:04:09 +0000
details:   https://anonhg.NetBSD.org/src/rev/ff5fdeaaaca8
branches:  trunk
changeset: 939004:ff5fdeaaaca8
user:      rillig <rillig%NetBSD.org@localhost>
date:      Wed Sep 23 07:50:58 2020 +0000

description:
make(1): fix unexpected behavior in ::= variable modifier

Previously, the ::= modifier had returned an error value, which caused
the variable expression to be preserved.  This behavior was not useful
in this case; it had only been intended to be used for undefined
variables.

To fix it, distinguish between parse errors, undefined variables and
regular empty strings.

diffstat:

 usr.bin/make/unit-tests/cond-func-empty.mk |   4 +-
 usr.bin/make/unit-tests/counter-append.exp |  33 +++----------
 usr.bin/make/unit-tests/counter-append.mk  |  16 ++----
 usr.bin/make/unit-tests/counter.exp        |  36 +++-----------
 usr.bin/make/unit-tests/counter.mk         |  16 ++----
 usr.bin/make/var.c                         |  72 +++++++++++++++---------------
 6 files changed, 66 insertions(+), 111 deletions(-)

diffs (truncated from 409 to 300 lines):

diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/unit-tests/cond-func-empty.mk
--- a/usr.bin/make/unit-tests/cond-func-empty.mk        Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/unit-tests/cond-func-empty.mk        Wed Sep 23 07:50:58 2020 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: cond-func-empty.mk,v 1.6 2020/09/04 21:08:44 rillig Exp $
+# $NetBSD: cond-func-empty.mk,v 1.7 2020/09/23 07:50:58 rillig Exp $
 #
 # Tests for the empty() function in .if conditions, which tests a variable
 # expression for emptiness.
@@ -116,7 +116,7 @@
 .endif
 
 # The expression ${} for a variable with the empty name always evaluates
-# to an empty string (see Var_Parse, varNoError).
+# to an empty string (see Var_Parse, varUndefined).
 .if !empty()
 .  error
 .endif
diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/unit-tests/counter-append.exp
--- a/usr.bin/make/unit-tests/counter-append.exp        Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/unit-tests/counter-append.exp        Wed Sep 23 07:50:58 2020 +0000
@@ -12,7 +12,7 @@
 Applying ${COUNTER:[...} to " a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "1" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:A = ${COUNTER::+=a}1
+Global:A = 1
 Global:B = 
 Var_Parse: ${NEXT} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER::+=a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
@@ -24,7 +24,7 @@
 Applying ${COUNTER:[...} to " a a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "2" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:B = ${COUNTER::+=a}2
+Global:B = 2
 Global:C = 
 Var_Parse: ${NEXT} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER::+=a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
@@ -36,45 +36,30 @@
 Applying ${COUNTER:[...} to " a a a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "3" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:C = ${COUNTER::+=a}3
+Global:C = 3
 Global:RELEVANT = no
 Global:RELEVANT = yes (run-time part)
 Result of ${RELEVANT::=yes (run-time part)} is "" (VARE_WANTRES, none, none)
 Var_Parse: ${A:Q} B=${B:Q} C=${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::+=a}1 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a a" (VARE_WANTRES, none, none)
-Modifier part: "a"
-Global:COUNTER =  a a a a
-Result of ${COUNTER::+=a} is "" (VARE_WANTRES, none, none)
 Applying ${A:Q} to "1" (VARE_WANTRES, none, none)
 QuoteMeta: [1]
 Result of ${A:Q} is "1" (VARE_WANTRES, none, none)
 Var_Parse: ${B:Q} C=${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::+=a}2 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a a a" (VARE_WANTRES, none, none)
-Modifier part: "a"
-Global:COUNTER =  a a a a a
-Result of ${COUNTER::+=a} is "" (VARE_WANTRES, none, none)
 Applying ${B:Q} to "2" (VARE_WANTRES, none, none)
 QuoteMeta: [2]
 Result of ${B:Q} is "2" (VARE_WANTRES, none, none)
 Var_Parse: ${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::+=a}3 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a a a a" (VARE_WANTRES, none, none)
-Modifier part: "a"
-Global:COUNTER =  a a a a a a
-Result of ${COUNTER::+=a} is "" (VARE_WANTRES, none, none)
 Applying ${C:Q} to "3" (VARE_WANTRES, none, none)
 QuoteMeta: [3]
 Result of ${C:Q} is "3" (VARE_WANTRES, none, none)
 Var_Parse: ${COUNTER:[#]:Q} with VARE_WANTRES
-Applying ${COUNTER:[...} to " a a a a a a" (VARE_WANTRES, none, none)
+Applying ${COUNTER:[...} to " a a a" (VARE_WANTRES, none, none)
 Modifier part: "#"
-Result of ${COUNTER:[#]} is "6" (VARE_WANTRES, none, none)
-Applying ${COUNTER:Q} to "6" (VARE_WANTRES, none, none)
-QuoteMeta: [6]
-Result of ${COUNTER:Q} is "6" (VARE_WANTRES, none, none)
-A=1 B=2 C=3 COUNTER=6
+Result of ${COUNTER:[#]} is "3" (VARE_WANTRES, none, none)
+Applying ${COUNTER:Q} to "3" (VARE_WANTRES, none, none)
+QuoteMeta: [3]
+Result of ${COUNTER:Q} is "3" (VARE_WANTRES, none, none)
+A=1 B=2 C=3 COUNTER=3
 Var_Parse: ${RELEVANT::=no} with VARE_WANTRES
 Applying ${RELEVANT::...} to "yes (run-time part)" (VARE_WANTRES, none, none)
 Modifier part: "no"
diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/unit-tests/counter-append.mk
--- a/usr.bin/make/unit-tests/counter-append.mk Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/unit-tests/counter-append.mk Wed Sep 23 07:50:58 2020 +0000
@@ -1,16 +1,10 @@
-# $NetBSD: counter-append.mk,v 1.1 2020/09/23 03:33:55 rillig Exp $
-#
-# Demonstrates that it is not easily possible to let make count
-# the number of times a variable is actually accessed, using the
-# ::+= variable modifier.
+# $NetBSD: counter-append.mk,v 1.2 2020/09/23 07:50:58 rillig Exp $
 #
-# As of 2020-09-23, the counter ends up at having 6 words, even
-# though the NEXT variable is only accessed 3 times.  This is
-# surprising.
+# Demonstrates how to let make count the number of times a variable
+# is actually accessed, using the ::+= variable modifier.
 #
-# A hint to this surprising behavior is that the variables don't
-# get fully expanded.  For example, A does not simply contain the
-# value "1" but an additional unexpanded ${COUNTER:...} before it.
+# This works since 2020-09-23.  Before that, the counter ended up at having
+# 6 words, even though the NEXT variable was only accessed 3 times.
 
 .MAKEFLAGS: -dv
 
diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/unit-tests/counter.exp
--- a/usr.bin/make/unit-tests/counter.exp       Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/unit-tests/counter.exp       Wed Sep 23 07:50:58 2020 +0000
@@ -9,12 +9,11 @@
 Modifier part: " a"
 Global:COUNTER =  a
 Result of ${COUNTER::=${COUNTER} a} is "" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Var_Parse: ${COUNTER} a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Applying ${COUNTER:[...} to " a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "1" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:A = ${COUNTER::= a a}1
+Global:A = 1
 Global:B = 
 Var_Parse: ${NEXT} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER::=${COUNTER} a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
@@ -23,12 +22,11 @@
 Modifier part: " a a"
 Global:COUNTER =  a a
 Result of ${COUNTER::=${COUNTER} a} is "" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Var_Parse: ${COUNTER} a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Applying ${COUNTER:[...} to " a a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "2" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:B = ${COUNTER::= a a a}2
+Global:B = 2
 Global:C = 
 Var_Parse: ${NEXT} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER::=${COUNTER} a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
@@ -37,50 +35,34 @@
 Modifier part: " a a a"
 Global:COUNTER =  a a a
 Result of ${COUNTER::=${COUNTER} a} is "" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Var_Parse: ${COUNTER} a}${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Var_Parse: ${COUNTER:[#]} with VARE_WANTRES|VARE_ASSIGN
 Applying ${COUNTER:[...} to " a a a" (VARE_WANTRES|VARE_ASSIGN, none, none)
 Modifier part: "#"
 Result of ${COUNTER:[#]} is "3" (VARE_WANTRES|VARE_ASSIGN, none, none)
-Global:C = ${COUNTER::= a a a a}3
+Global:C = 3
 Global:RELEVANT = no
 Global:RELEVANT = yes (run-time part)
 Result of ${RELEVANT::=yes (run-time part)} is "" (VARE_WANTRES, none, none)
 Var_Parse: ${A:Q} B=${B:Q} C=${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::= a a}1 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a a" (VARE_WANTRES, none, none)
-Modifier part: " a a"
-Global:COUNTER =  a a
-Result of ${COUNTER::= a a} is "" (VARE_WANTRES, none, none)
 Applying ${A:Q} to "1" (VARE_WANTRES, none, none)
 QuoteMeta: [1]
 Result of ${A:Q} is "1" (VARE_WANTRES, none, none)
 Var_Parse: ${B:Q} C=${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::= a a a}2 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a" (VARE_WANTRES, none, none)
-Modifier part: " a a a"
-Global:COUNTER =  a a a
-Result of ${COUNTER::= a a a} is "" (VARE_WANTRES, none, none)
 Applying ${B:Q} to "2" (VARE_WANTRES, none, none)
 QuoteMeta: [2]
 Result of ${B:Q} is "2" (VARE_WANTRES, none, none)
 Var_Parse: ${C:Q} COUNTER=${COUNTER:[#]:Q} with VARE_WANTRES
-Var_Parse: ${COUNTER::= a a a a}3 with VARE_WANTRES
-Applying ${COUNTER::...} to " a a a" (VARE_WANTRES, none, none)
-Modifier part: " a a a a"
-Global:COUNTER =  a a a a
-Result of ${COUNTER::= a a a a} is "" (VARE_WANTRES, none, none)
 Applying ${C:Q} to "3" (VARE_WANTRES, none, none)
 QuoteMeta: [3]
 Result of ${C:Q} is "3" (VARE_WANTRES, none, none)
 Var_Parse: ${COUNTER:[#]:Q} with VARE_WANTRES
-Applying ${COUNTER:[...} to " a a a a" (VARE_WANTRES, none, none)
+Applying ${COUNTER:[...} to " a a a" (VARE_WANTRES, none, none)
 Modifier part: "#"
-Result of ${COUNTER:[#]} is "4" (VARE_WANTRES, none, none)
-Applying ${COUNTER:Q} to "4" (VARE_WANTRES, none, none)
-QuoteMeta: [4]
-Result of ${COUNTER:Q} is "4" (VARE_WANTRES, none, none)
-A=1 B=2 C=3 COUNTER=4
+Result of ${COUNTER:[#]} is "3" (VARE_WANTRES, none, none)
+Applying ${COUNTER:Q} to "3" (VARE_WANTRES, none, none)
+QuoteMeta: [3]
+Result of ${COUNTER:Q} is "3" (VARE_WANTRES, none, none)
+A=1 B=2 C=3 COUNTER=3
 Var_Parse: ${RELEVANT::=no} with VARE_WANTRES
 Applying ${RELEVANT::...} to "yes (run-time part)" (VARE_WANTRES, none, none)
 Modifier part: "no"
diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/unit-tests/counter.mk
--- a/usr.bin/make/unit-tests/counter.mk        Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/unit-tests/counter.mk        Wed Sep 23 07:50:58 2020 +0000
@@ -1,16 +1,10 @@
-# $NetBSD: counter.mk,v 1.2 2020/09/23 03:33:55 rillig Exp $
-#
-# Demonstrates that it is not easily possible to let make count
-# the number of times a variable is actually accessed, using the
-# ::= variable modifier.
+# $NetBSD: counter.mk,v 1.3 2020/09/23 07:50:58 rillig Exp $
 #
-# As of 2020-08-02, the counter ends up at having 4 words, even
-# though the NEXT variable is only accessed 3 times.  This is
-# surprising.
+# Demonstrates how to let make count the number of times a variable
+# is actually accessed, using the ::= variable modifier.
 #
-# A hint to this surprising behavior is that the variables don't
-# get fully expanded.  For example, A does not simply contain the
-# value "1" but an additional unexpanded ${COUNTER:...} before it.
+# This works since 2020-09-23.  Before that, the counter ended up at having
+# 4 words, even though the NEXT variable was only accessed 3 times.
 
 .MAKEFLAGS: -dv
 
diff -r ad64485040c9 -r ff5fdeaaaca8 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Wed Sep 23 07:30:12 2020 +0000
+++ b/usr.bin/make/var.c        Wed Sep 23 07:50:58 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.535 2020/09/23 04:27:39 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.536 2020/09/23 07:50:58 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -121,7 +121,7 @@
 #include    "metachar.h"
 
 /*     "@(#)var.c      8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.535 2020/09/23 04:27:39 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.536 2020/09/23 07:50:58 rillig Exp $");
 
 #define VAR_DEBUG_IF(cond, fmt, ...)   \
     if (!(DEBUG(VAR) && (cond)))       \
@@ -140,21 +140,21 @@
  */
 char **savedEnv = NULL;
 
-/*
- * This is a harmless return value for Var_Parse that can be used by Var_Subst
- * to determine if there was an error in parsing -- easier than returning
- * a flag, as things outside this module don't give a hoot.
- */
+/* Special return value for Var_Parse, indicating a parse error.  It may be
+ * caused by an undefined variable, a syntax error in a modifier or
+ * something entirely different. */
 char var_Error[] = "";
 
-/*
- * Similar to var_Error, but returned when the 'VARE_UNDEFERR' flag for
- * Var_Parse is not set.
- *
- * Why not just use a constant? Well, GCC likes to condense identical string
- * instances...
- */
-static char varNoError[] = "";
+/* Special return value for Var_Parse, indicating an undefined variable in
+ * a case where VARE_UNDEFERR is not set.  This undefined variable is
+ * typically a dynamic variable such as ${.TARGET}, whose expansion needs to
+ * be deferred until it is defined in an actual target. */
+static char varUndefined[] = "";
+
+/* Special return value for Var_Parse, just to avoid allocating empty strings.
+ * In contrast to var_Error and varUndefined, this is not an error marker but
+ * just an ordinary successful return value. */
+static char emptyString[] = "";
 
 /*
  * Traditionally we consume $$ during := like any other expansion.
@@ -2171,7 +2171,7 @@
     if (st->eflags & VARE_WANTRES)
        st->newVal = Cmd_Exec(cmd, &errfmt);
     else
-       st->newVal = varNoError;
+       st->newVal = emptyString;
     free(cmd);
 
     if (errfmt != NULL)
@@ -2872,9 +2872,7 @@
        }
     }
     free(val);
-    st->newVal = varNoError;   /* XXX: varNoError is kind of an error,
-                                * the intention here is to just return
-                                * an empty string. */
Prev by Date: [src/trunk]: src/usr.bin/make make(1): merge duplicate code for delayed commands
Next by Date: [src/trunk]: src/usr.bin/make/unit-tests make(1): remove debug output from co...
Previous by Thread: [src/trunk]: src/usr.bin/make make(1): merge duplicate code for delayed commands
Next by Thread: [src/trunk]: src/usr.bin/make/unit-tests make(1): remove debug output from co...
Indexes:
Home | Main Index | Thread Index | Old Index