Module Name:    src
Committed By:   rillig
Date:           Tue Aug 23 19:22:02 UTC 2022

Modified Files:
        src/usr.bin/make: var.c
        src/usr.bin/make/unit-tests: parse-var.exp parse-var.mk
            var-eval-short.exp varmod-defined.exp varmod-loop.exp
            varmod-loop.mk varname-dot-suffixes.exp

Log Message:
make: revert parsing of modifier parts (since 2022-08-08)

The modifier ':@var@body@' parses the body in parse-only mode and later
uses Var_Subst on it, in which each literal '$' must be written as '$$'.

Trying to parse the loop body using Var_Parse treated the text
'$${var:-0}' as a single '$' followed by the expression '${var:-0}',
wrongly complaining about the 'Unknown modifier "-0"'.

Found by sjg.


To generate a diff of this commit:
cvs rdiff -u -r1.1028 -r1.1029 src/usr.bin/make/var.c
cvs rdiff -u -r1.3 -r1.4 src/usr.bin/make/unit-tests/parse-var.exp
cvs rdiff -u -r1.4 -r1.5 src/usr.bin/make/unit-tests/parse-var.mk \
    src/usr.bin/make/unit-tests/varname-dot-suffixes.exp
cvs rdiff -u -r1.19 -r1.20 src/usr.bin/make/unit-tests/var-eval-short.exp \
    src/usr.bin/make/unit-tests/varmod-loop.mk
cvs rdiff -u -r1.12 -r1.13 src/usr.bin/make/unit-tests/varmod-defined.exp
cvs rdiff -u -r1.16 -r1.17 src/usr.bin/make/unit-tests/varmod-loop.exp

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.1028 src/usr.bin/make/var.c:1.1029
--- src/usr.bin/make/var.c:1.1028	Mon Aug  8 18:23:30 2022
+++ src/usr.bin/make/var.c	Tue Aug 23 19:22:01 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: var.c,v 1.1028 2022/08/08 18:23:30 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.1029 2022/08/23 19:22:01 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -139,7 +139,7 @@
 #include "metachar.h"
 
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.1028 2022/08/08 18:23:30 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.1029 2022/08/23 19:22:01 rillig Exp $");
 
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
@@ -2131,33 +2131,49 @@ ParseModifierPartExpr(const char **pp, L
 	*pp = p;
 }
 
-/* In a part of a modifier, parse a subexpression but don't evaluate it. */
+/*
+ * In a part of a modifier, parse a subexpression but don't evaluate it.
+ *
+ * XXX: This whole block is very similar to Var_Parse with VARE_PARSE_ONLY.
+ * There may be subtle edge cases though that are not yet covered in the unit
+ * tests and that are parsed differently, depending on whether they are
+ * evaluated or not.
+ *
+ * This subtle difference is not documented in the manual page, neither is
+ * the difference between parsing ':D' and ':M' documented.  No code should
+ * ever depend on these details, but who knows.
+ *
+ * TODO: Before trying to replace this code with Var_Parse, there need to be
+ * more unit tests in varmod-loop.mk.  The modifier ':@' uses Var_Subst
+ * internally, in which a '$' is escaped as '$$', not as '\$' like in other
+ * modifiers.  When parsing the body text '$${var}', skipping over the first
+ * '$' would treat '${var}' as a make expression, not as a shell variable.
+ */
 static void
 ParseModifierPartDollar(const char **pp, LazyBuf *part)
 {
 	const char *p = *pp;
+	const char *start = *pp;
 
 	if (p[1] == '(' || p[1] == '{') {
-		FStr unused;
-		Var_Parse(&p, SCOPE_GLOBAL, VARE_PARSE_ONLY, &unused);
-		/* TODO: handle errors */
-		FStr_Done(&unused);
+		char startc = p[1];
+		int endc = startc == '(' ? ')' : '}';
+		int depth = 1;
+
+		for (p += 2; *p != '\0' && depth > 0; p++) {
+			if (p[-1] != '\\') {
+				if (*p == startc)
+					depth++;
+				if (*p == endc)
+					depth--;
+			}
+		}
+		LazyBuf_AddBytesBetween(part, start, p);
+		*pp = p;
 	} else {
-		/*
-		 * Only skip the '$' but not the next character; see
-		 * ParseModifierPartSubst, the case for "Unescaped '$' at
-		 * end", which also doesn't skip '$' + delimiter.  That is a
-		 * hack as well, but for now it's consistent in both cases.
-		 */
-		p++;
+		LazyBuf_Add(part, *start);
+		*pp = p + 1;
 	}
-
-	/*
-	 * XXX: There should be no need to add anything to the buffer, as it
-	 * will be discarded anyway.
-	 */
-	LazyBuf_AddBytesBetween(part, *pp, p);
-	*pp = p;
 }
 
 /* See ParseModifierPart for the documentation. */

Index: src/usr.bin/make/unit-tests/parse-var.exp
diff -u src/usr.bin/make/unit-tests/parse-var.exp:1.3 src/usr.bin/make/unit-tests/parse-var.exp:1.4
--- src/usr.bin/make/unit-tests/parse-var.exp:1.3	Mon Aug  8 18:23:30 2022
+++ src/usr.bin/make/unit-tests/parse-var.exp	Tue Aug 23 19:22:01 2022
@@ -1 +1,5 @@
-exit status 0
+make: Unfinished modifier for "BRACE_GROUP" (',' missing)
+make: "parse-var.mk" line 57: Malformed conditional (0 && ${BRACE_GROUP:S,${BRACE_PAIR:S,{,{{,},<lbraces>,})
+make: Fatal errors encountered -- cannot continue
+make: stopped in unit-tests
+exit status 1

Index: src/usr.bin/make/unit-tests/parse-var.mk
diff -u src/usr.bin/make/unit-tests/parse-var.mk:1.4 src/usr.bin/make/unit-tests/parse-var.mk:1.5
--- src/usr.bin/make/unit-tests/parse-var.mk:1.4	Mon Aug  8 19:53:28 2022
+++ src/usr.bin/make/unit-tests/parse-var.mk	Tue Aug 23 19:22:01 2022
@@ -1,4 +1,4 @@
-# $NetBSD: parse-var.mk,v 1.4 2022/08/08 19:53:28 rillig Exp $
+# $NetBSD: parse-var.mk,v 1.5 2022/08/23 19:22:01 rillig Exp $
 #
 # Tests for parsing variable expressions.
 
@@ -13,8 +13,10 @@ VAR.${:U param }=	value
 .  error
 .endif
 
+# XXX: The following paragraph already uses past tense, in the hope that the
+# parsing behavior can be cleaned up soon.
 
-# Since var.c 1.323 from 202-07-26 18:11 and before var.c 1.1028 from
+# Since var.c 1.323 from 2020-07-26 18:11 and except for var.c 1.1028 from
 # 2022-08-08, the exact way of parsing an expression depended on whether the
 # expression was actually evaluated or merely parsed.
 #
@@ -30,7 +32,7 @@ VAR.${:U param }=	value
 # braces.
 #
 # This naive brace counting was implemented in ParseModifierPartDollar.  As of
-# var.c 1., there are still several other places that merely count braces
+# var.c 1.1029, there are still several other places that merely count braces
 # instead of properly parsing subexpressions.
 
 #.MAKEFLAGS: -dcpv
@@ -51,7 +53,7 @@ BRACE_GROUP=	{{{{}}}}
 # the parts of the outer ':S' modifier, make only counted the braces, and since
 # the inner expression '${BRACE_PAIR:...}' contains more '{' than '}', parsing
 # failed with the error message 'Unfinished modifier for "BRACE_GROUP"'.  Fixed
-# in var.c 1.1028 from 2022-08-08.
+# in var.c 1.1028 from 2022-08-08, reverted in var.c 1.1029 from 2022-08-23.
 .if 0 && ${BRACE_GROUP:S,${BRACE_PAIR:S,{,{{,},<lbraces>,}
 .endif
 #.MAKEFLAGS: -d0
Index: src/usr.bin/make/unit-tests/varname-dot-suffixes.exp
diff -u src/usr.bin/make/unit-tests/varname-dot-suffixes.exp:1.4 src/usr.bin/make/unit-tests/varname-dot-suffixes.exp:1.5
--- src/usr.bin/make/unit-tests/varname-dot-suffixes.exp:1.4	Mon Aug  8 18:23:30 2022
+++ src/usr.bin/make/unit-tests/varname-dot-suffixes.exp	Tue Aug 23 19:22:01 2022
@@ -24,7 +24,6 @@ Evaluating modifier ${1 2:L} on value ""
 Result of ${1 2:L} is "1 2" (eval-defined, defined)
 Evaluating modifier ${1 2:@...} on value "1 2" (eval-defined, defined)
 Modifier part: ".SUFFIXES"
-Var_Parse: ${.SUFFIXES}@} != ".c .o .1 .err .tar.gz .c .o .1 .err .tar.gz" (parse-only)
 Modifier part: "${.SUFFIXES}"
 ModifyWords: split "1 2" into 2 words
 Command: .SUFFIXES = 1 ignored (read-only)

Index: src/usr.bin/make/unit-tests/var-eval-short.exp
diff -u src/usr.bin/make/unit-tests/var-eval-short.exp:1.19 src/usr.bin/make/unit-tests/var-eval-short.exp:1.20
--- src/usr.bin/make/unit-tests/var-eval-short.exp:1.19	Mon Aug  8 18:23:30 2022
+++ src/usr.bin/make/unit-tests/var-eval-short.exp	Tue Aug 23 19:22:01 2022
@@ -7,9 +7,7 @@ make: "var-eval-short.mk" line 98: Malfo
 CondParser_Eval: 0 && ${0:?${FAIL}then:${FAIL}else}
 Var_Parse: ${0:?${FAIL}then:${FAIL}else} (parse-only)
 Parsing modifier ${0:?...}
-Var_Parse: ${FAIL}then:${FAIL}else} (parse-only)
 Modifier part: "${FAIL}then"
-Var_Parse: ${FAIL}else} (parse-only)
 Modifier part: "${FAIL}else"
 Result of ${0:?${FAIL}then:${FAIL}else} is "" (parse-only, defined)
 Parsing line 163: DEFINED=	defined
@@ -19,9 +17,7 @@ Var_Parse: ${DEFINED:L:?${FAIL}then:${FA
 Parsing modifier ${DEFINED:L}
 Result of ${DEFINED:L} is "defined" (parse-only, regular)
 Parsing modifier ${DEFINED:?...}
-Var_Parse: ${FAIL}then:${FAIL}else} (parse-only)
 Modifier part: "${FAIL}then"
-Var_Parse: ${FAIL}else} (parse-only)
 Modifier part: "${FAIL}else"
 Result of ${DEFINED:?${FAIL}then:${FAIL}else} is "defined" (parse-only, regular)
 Parsing line 166: .MAKEFLAGS: -d0
Index: src/usr.bin/make/unit-tests/varmod-loop.mk
diff -u src/usr.bin/make/unit-tests/varmod-loop.mk:1.19 src/usr.bin/make/unit-tests/varmod-loop.mk:1.20
--- src/usr.bin/make/unit-tests/varmod-loop.mk:1.19	Tue Aug 23 17:40:43 2022
+++ src/usr.bin/make/unit-tests/varmod-loop.mk	Tue Aug 23 19:22:01 2022
@@ -1,4 +1,4 @@
-# $NetBSD: varmod-loop.mk,v 1.19 2022/08/23 17:40:43 rillig Exp $
+# $NetBSD: varmod-loop.mk,v 1.20 2022/08/23 19:22:01 rillig Exp $
 #
 # Tests for the :@var@...${var}...@ variable modifier.
 
@@ -192,7 +192,8 @@ CMDLINE=	global		# needed for deleting t
 # except for '$i', which is replaced with the then-current value '1' of the
 # iteration variable.
 #
-# FIXME: broken since var.c 1.1028 from 2022-08-08.
+# XXX: was broken in var.c 1.1028 from 2022-08-08, reverted in var.c 1.1029
+# from 2022-08-23; see parse-var.mk, keyword 'BRACE_GROUP'.
 all: varmod-loop-literal-dollar
 varmod-loop-literal-dollar: .PHONY
 	: ${:U1:@i@ t=$$(( $${t:-0} + $i ))@}

Index: src/usr.bin/make/unit-tests/varmod-defined.exp
diff -u src/usr.bin/make/unit-tests/varmod-defined.exp:1.12 src/usr.bin/make/unit-tests/varmod-defined.exp:1.13
--- src/usr.bin/make/unit-tests/varmod-defined.exp:1.12	Mon Aug  8 18:23:30 2022
+++ src/usr.bin/make/unit-tests/varmod-defined.exp	Tue Aug 23 19:22:01 2022
@@ -10,7 +10,6 @@ Global: VAR = $$$$$$$$
 Var_Parse: ${VAR:@var@${8_DOLLARS}@} (eval-keep-dollar-and-undefined)
 Evaluating modifier ${VAR:@...} on value "$$$$$$$$" (eval-keep-dollar-and-undefined, regular)
 Modifier part: "var"
-Var_Parse: ${8_DOLLARS}@} (parse-only)
 Modifier part: "${8_DOLLARS}"
 ModifyWords: split "$$$$$$$$" into 1 word
 Global: var = $$$$$$$$

Index: src/usr.bin/make/unit-tests/varmod-loop.exp
diff -u src/usr.bin/make/unit-tests/varmod-loop.exp:1.16 src/usr.bin/make/unit-tests/varmod-loop.exp:1.17
--- src/usr.bin/make/unit-tests/varmod-loop.exp:1.16	Tue Aug 23 17:40:43 2022
+++ src/usr.bin/make/unit-tests/varmod-loop.exp	Tue Aug 23 19:22:01 2022
@@ -13,6 +13,5 @@ mod-loop-dollar:$3$:
 mod-loop-dollar:$${word}$$:
 mod-loop-dollar:$$5$$:
 mod-loop-dollar:$$${word}$$$:
-make: Unknown modifier "-0"
 :  t=$(( ${t:-0} + 1 ))
 exit status 0

Reply via email to