Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/make make: revert parsing of modifier parts (since 2...



details:   https://anonhg.NetBSD.org/src/rev/634a6ca887e3
branches:  trunk
changeset: 369614:634a6ca887e3
user:      rillig <rillig%NetBSD.org@localhost>
date:      Tue Aug 23 19:22:01 2022 +0000

description:
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.

diffstat:

 usr.bin/make/unit-tests/parse-var.exp            |   6 ++-
 usr.bin/make/unit-tests/parse-var.mk             |  10 ++-
 usr.bin/make/unit-tests/var-eval-short.exp       |   4 -
 usr.bin/make/unit-tests/varmod-defined.exp       |   1 -
 usr.bin/make/unit-tests/varmod-loop.exp          |   1 -
 usr.bin/make/unit-tests/varmod-loop.mk           |   5 +-
 usr.bin/make/unit-tests/varname-dot-suffixes.exp |   1 -
 usr.bin/make/var.c                               |  58 +++++++++++++++--------
 8 files changed, 51 insertions(+), 35 deletions(-)

diffs (210 lines):

diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/parse-var.exp
--- a/usr.bin/make/unit-tests/parse-var.exp     Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/parse-var.exp     Tue Aug 23 19:22:01 2022 +0000
@@ -1,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
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/parse-var.mk
--- a/usr.bin/make/unit-tests/parse-var.mk      Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/parse-var.mk      Tue Aug 23 19:22:01 2022 +0000
@@ -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 @@
 .  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 @@
 # 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 @@
 # 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
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/var-eval-short.exp
--- a/usr.bin/make/unit-tests/var-eval-short.exp        Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/var-eval-short.exp        Tue Aug 23 19:22:01 2022 +0000
@@ -7,9 +7,7 @@
 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 @@
 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
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/varmod-defined.exp
--- a/usr.bin/make/unit-tests/varmod-defined.exp        Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/varmod-defined.exp        Tue Aug 23 19:22:01 2022 +0000
@@ -10,7 +10,6 @@
 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 = $$$$$$$$
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/varmod-loop.exp
--- a/usr.bin/make/unit-tests/varmod-loop.exp   Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/varmod-loop.exp   Tue Aug 23 19:22:01 2022 +0000
@@ -13,6 +13,5 @@
 mod-loop-dollar:$${word}$$:
 mod-loop-dollar:$$5$$:
 mod-loop-dollar:$$${word}$$$:
-make: Unknown modifier "-0"
 :  t=$(( ${t:-0} + 1 ))
 exit status 0
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/varmod-loop.mk
--- a/usr.bin/make/unit-tests/varmod-loop.mk    Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/varmod-loop.mk    Tue Aug 23 19:22:01 2022 +0000
@@ -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 @@
 # 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 ))@}
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/unit-tests/varname-dot-suffixes.exp
--- a/usr.bin/make/unit-tests/varname-dot-suffixes.exp  Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/unit-tests/varname-dot-suffixes.exp  Tue Aug 23 19:22:01 2022 +0000
@@ -24,7 +24,6 @@
 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)
diff -r 8cef8305bde8 -r 634a6ca887e3 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Tue Aug 23 17:40:43 2022 +0000
+++ b/usr.bin/make/var.c        Tue Aug 23 19:22:01 2022 +0000
@@ -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 @@
        *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. */



Home | Main Index | Thread Index | Old Index