Module Name: src
Committed By: rillig
Date: Wed Sep 23 07:50:59 UTC 2020
Modified Files:
src/usr.bin/make: var.c
src/usr.bin/make/unit-tests: cond-func-empty.mk counter-append.exp
counter-append.mk counter.exp counter.mk
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.535 -r1.536 src/usr.bin/make/var.c
cvs rdiff -u -r1.6 -r1.7 src/usr.bin/make/unit-tests/cond-func-empty.mk \
src/usr.bin/make/unit-tests/counter.exp
cvs rdiff -u -r1.1 -r1.2 src/usr.bin/make/unit-tests/counter-append.exp \
src/usr.bin/make/unit-tests/counter-append.mk
cvs rdiff -u -r1.2 -r1.3 src/usr.bin/make/unit-tests/counter.mk
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/make/var.c
diff -u src/usr.bin/make/var.c:1.535 src/usr.bin/make/var.c:1.536
--- src/usr.bin/make/var.c:1.535 Wed Sep 23 04:27:39 2020
+++ src/usr.bin/make/var.c Wed Sep 23 07:50:58 2020
@@ -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 @@ ENUM_FLAGS_RTTI_3(VarEvalFlags,
*/
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 @@ ApplyModifier_ShellCommand(const char **
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 @@ ApplyModifier_Assign(const char **pp, Ap
}
}
free(val);
- st->newVal = varNoError; /* XXX: varNoError is kind of an error,
- * the intention here is to just return
- * an empty string. */
+ st->newVal = emptyString;
return AMR_OK;
}
@@ -3040,7 +3038,7 @@ ApplyModifiers(
st.val = ApplyModifiers(&rval_pp, st.val, '\0', '\0', v,
exprFlags, ctxt, eflags, freePtr);
if (st.val == var_Error
- || (st.val == varNoError && !(st.eflags & VARE_UNDEFERR))
+ || (st.val == varUndefined && !(st.eflags & VARE_UNDEFERR))
|| *rval_pp != '\0') {
free(freeIt);
goto out; /* error already reported */
@@ -3181,7 +3179,7 @@ ApplyModifiers(
if (errfmt)
Error(errfmt, st.val);
} else
- st.newVal = varNoError;
+ st.newVal = emptyString;
p += 2;
res = AMR_OK;
} else
@@ -3234,7 +3232,8 @@ ApplyModifiers(
*freePtr = NULL;
}
st.val = st.newVal;
- if (st.val != var_Error && st.val != varNoError) {
+ if (st.val != var_Error && st.val != varUndefined &&
+ st.val != emptyString) {
*freePtr = st.val;
}
}
@@ -3249,7 +3248,7 @@ ApplyModifiers(
}
out:
*pp = p;
- assert(st.val != NULL); /* Use var_Error or varNoError instead. */
+ assert(st.val != NULL); /* Use var_Error or varUndefined instead. */
*exprFlags = st.exprFlags;
return st.val;
@@ -3327,7 +3326,7 @@ ShortVarValue(char varname, const GNode
return "$(.ARCHIVE)";
}
}
- return eflags & VARE_UNDEFERR ? var_Error : varNoError;
+ return eflags & VARE_UNDEFERR ? var_Error : varUndefined;
}
/* Parse a variable name, until the end character or a colon, whichever
@@ -3420,20 +3419,21 @@ ValidShortVarname(char varname, const ch
*
* Results:
* Returns the value of the variable expression, never NULL.
- * var_Error if there was a parse error and VARE_UNDEFERR was set.
- * varNoError if there was a parse error and VARE_UNDEFERR was not set.
- *
- * Parsing should continue at str + *lengthPtr.
- * TODO: Document the value of *lengthPtr on parse errors. It might be
- * 0, or +1, or the index of the parse error, or the guessed end of the
+ * Returns var_Error if there was a parse error and VARE_UNDEFERR was
+ * set.
+ * Returns varUndefined if there was an undefined variable and
+ * VARE_UNDEFERR was not set.
+ *
+ * Parsing should continue at *pp.
+ * TODO: Document the value of *pp on parse errors. It might be advanced
+ * by 0, or +1, or the index of the parse error, or the guessed end of the
* variable expression.
*
* If var_Error is returned, a diagnostic may or may not have been
* printed. XXX: This is inconsistent.
*
- * If varNoError is returned, a diagnostic may or may not have been
- * printed. XXX: This is inconsistent, and as of 2020-09-08, returning
- * varNoError is even used to return a regular, non-error empty string.
+ * If varUndefined is returned, a diagnostic may or may not have been
+ * printed. XXX: This is inconsistent.
*
* After using the returned value, *freePtr must be freed, preferably
* using bmake_free since it is NULL in most cases.
@@ -3588,7 +3588,7 @@ Var_Parse(const char **pp, GNode *ctxt,
}
free(varname);
- *out_val = varNoError;
+ *out_val = varUndefined;
return VPR_OK;
}
@@ -3686,8 +3686,8 @@ Var_Parse(const char **pp, GNode *ctxt,
*freePtr = nstr;
} else {
/* The expression is still undefined, therefore discard the
- * actual value and return an empty string instead. */
- nstr = (eflags & VARE_UNDEFERR) ? var_Error : varNoError;
+ * actual value and return an error marker instead. */
+ nstr = (eflags & VARE_UNDEFERR) ? var_Error : varUndefined;
}
}
if (nstr != Buf_GetAll(&v->val, NULL))
@@ -3757,7 +3757,7 @@ Var_Subst(const char *str, GNode *ctxt,
(void)Var_Parse(&nested_str, ctxt, eflags, &val, &freeIt);
/* TODO: handle errors */
- if (val == var_Error || val == varNoError) {
+ if (val == var_Error || val == varUndefined) {
/*
* If performing old-time variable substitution, skip over
* the variable and continue with the substitution. Otherwise,
Index: src/usr.bin/make/unit-tests/cond-func-empty.mk
diff -u src/usr.bin/make/unit-tests/cond-func-empty.mk:1.6 src/usr.bin/make/unit-tests/cond-func-empty.mk:1.7
--- src/usr.bin/make/unit-tests/cond-func-empty.mk:1.6 Fri Sep 4 21:08:44 2020
+++ src/usr.bin/make/unit-tests/cond-func-empty.mk Wed Sep 23 07:50:58 2020
@@ -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 @@ ${:U }= space
.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
Index: src/usr.bin/make/unit-tests/counter.exp
diff -u src/usr.bin/make/unit-tests/counter.exp:1.6 src/usr.bin/make/unit-tests/counter.exp:1.7
--- src/usr.bin/make/unit-tests/counter.exp:1.6 Tue Sep 22 06:13:39 2020
+++ src/usr.bin/make/unit-tests/counter.exp Wed Sep 23 07:50:58 2020
@@ -9,12 +9,11 @@ Var_Parse: ${COUNTER} a}${COUNTER:[#]} w
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 @@ Var_Parse: ${COUNTER} a}${COUNTER:[#]} w
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 @@ Var_Parse: ${COUNTER} a}${COUNTER:[#]} w
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"
Index: src/usr.bin/make/unit-tests/counter-append.exp
diff -u src/usr.bin/make/unit-tests/counter-append.exp:1.1 src/usr.bin/make/unit-tests/counter-append.exp:1.2
--- src/usr.bin/make/unit-tests/counter-append.exp:1.1 Wed Sep 23 03:33:55 2020
+++ src/usr.bin/make/unit-tests/counter-append.exp Wed Sep 23 07:50:58 2020
@@ -12,7 +12,7 @@ Var_Parse: ${COUNTER:[#]} with VARE_WANT
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 @@ Var_Parse: ${COUNTER:[#]} with VARE_WANT
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 @@ Var_Parse: ${COUNTER:[#]} with VARE_WANT
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"
Index: src/usr.bin/make/unit-tests/counter-append.mk
diff -u src/usr.bin/make/unit-tests/counter-append.mk:1.1 src/usr.bin/make/unit-tests/counter-append.mk:1.2
--- src/usr.bin/make/unit-tests/counter-append.mk:1.1 Wed Sep 23 03:33:55 2020
+++ src/usr.bin/make/unit-tests/counter-append.mk Wed Sep 23 07:50:58 2020
@@ -1,16 +1,10 @@
-# $NetBSD: counter-append.mk,v 1.1 2020/09/23 03:33:55 rillig Exp $
+# $NetBSD: counter-append.mk,v 1.2 2020/09/23 07:50:58 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.
+# Demonstrates how to let make count the number of times a variable
+# is actually accessed, using the ::+= variable modifier.
#
-# 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.
-#
-# 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
Index: src/usr.bin/make/unit-tests/counter.mk
diff -u src/usr.bin/make/unit-tests/counter.mk:1.2 src/usr.bin/make/unit-tests/counter.mk:1.3
--- src/usr.bin/make/unit-tests/counter.mk:1.2 Wed Sep 23 03:33:55 2020
+++ src/usr.bin/make/unit-tests/counter.mk Wed Sep 23 07:50:58 2020
@@ -1,16 +1,10 @@
-# $NetBSD: counter.mk,v 1.2 2020/09/23 03:33:55 rillig Exp $
+# $NetBSD: counter.mk,v 1.3 2020/09/23 07:50:58 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.
+# Demonstrates how to let make count the number of times a variable
+# is actually accessed, using the ::= variable modifier.
#
-# 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.
-#
-# 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
