[src/trunk]: src/usr.bin/make make(1): make modifier handling simpler

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/b9f802f849f7
branches:  trunk
changeset: 974015:b9f802f849f7
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon Jul 20 14:50:41 2020 +0000

description:
make(1): make modifier handling simpler

Implementing a modifier such as :S or :M should not be concerned with
separating the words of the resulting string.  Ideally this should be
done in the same way by all modifiers.

Before, the :R (filename root) modifier added a separator even if the
resulting filename root was an empty string.  The chances that this
change in behavior breaks anything are epsilon.

The :@ modifier, if it appeared after a :ts modifier, did not use the
word separator from the :ts modifier (which all other modifiers do) but
always added a space.  This behavior has been preserved for now.  It's an
unnecessary inconsistency though.

In contrast to Buffer, the newly added SepBuf uses size_t for memory
sizes and also uses the conventional parameter order (mem, memsize)
instead of the unusual (memsize, mem).

diffstat:

 usr.bin/make/parse.c                |   11 +-
 usr.bin/make/unit-tests/modmisc.exp |    2 +-
 usr.bin/make/var.c                  |  424 ++++++++++++++---------------------
 3 files changed, 171 insertions(+), 266 deletions(-)

diffs (truncated from 849 to 300 lines):

diff -r 527ffb1339be -r b9f802f849f7 usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Mon Jul 20 14:38:38 2020 +0000
+++ b/usr.bin/make/parse.c      Mon Jul 20 14:50:41 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.237 2020/07/19 12:26:17 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.238 2020/07/20 14:50:41 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
  */
 
 #ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: parse.c,v 1.237 2020/07/19 12:26:17 rillig Exp $";
+static char rcsid[] = "$NetBSD: parse.c,v 1.238 2020/07/20 14:50:41 rillig Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)parse.c    8.3 (Berkeley) 3/19/94";
 #else
-__RCSID("$NetBSD: parse.c,v 1.237 2020/07/19 12:26:17 rillig Exp $");
+__RCSID("$NetBSD: parse.c,v 1.238 2020/07/20 14:50:41 rillig Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -1938,9 +1938,8 @@
            break;
     }
 
-    while (isspace ((unsigned char)*cp)) {
+    while (isspace((unsigned char)*cp))
        cp++;
-    }
 
     if (type == VAR_APPEND) {
        Var_Append(line, cp, ctxt);
@@ -1980,7 +1979,7 @@
            /*
             * There's a dollar sign in the command, so perform variable
             * expansion on the whole thing. The resulting string will need
-            * freeing when we're done, so set freeCmd to TRUE.
+            * freeing when we're done.
             */
            cp = Var_Subst(NULL, cp, VAR_CMD, VARE_UNDEFERR|VARE_WANTRES);
            freeCp = TRUE;
diff -r 527ffb1339be -r b9f802f849f7 usr.bin/make/unit-tests/modmisc.exp
--- a/usr.bin/make/unit-tests/modmisc.exp       Mon Jul 20 14:38:38 2020 +0000
+++ b/usr.bin/make/unit-tests/modmisc.exp       Mon Jul 20 14:50:41 2020 +0000
@@ -10,7 +10,7 @@
 dirname of 'a/b/c def a.b.c a.b/c a a.a .gitignore a a.a' is 'a/b . . a.b . . . . .'
 basename of 'a/b/c def a.b.c a.b/c a a.a .gitignore a a.a' is 'c def a.b.c c a a.a .gitignore a a.a'
 suffix of 'a/b/c def a.b.c a.b/c a a.a .gitignore a a.a' is 'c b/c a gitignore a'
-root of 'a/b/c def a.b.c a.b/c a a.a .gitignore a a.a' is 'a/b/c def a.b a a a  a a'
+root of 'a/b/c def a.b.c a.b/c a a.a .gitignore a a.a' is 'a/b/c def a.b a a a a a'
 S:
 C:
 @:
diff -r 527ffb1339be -r b9f802f849f7 usr.bin/make/var.c
--- a/usr.bin/make/var.c        Mon Jul 20 14:38:38 2020 +0000
+++ b/usr.bin/make/var.c        Mon Jul 20 14:50:41 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: var.c,v 1.277 2020/07/19 22:22:01 rillig Exp $ */
+/*     $NetBSD: var.c,v 1.278 2020/07/20 14:50:41 rillig Exp $ */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -69,14 +69,14 @@
  */
 
 #ifndef MAKE_NATIVE
-static char rcsid[] = "$NetBSD: var.c,v 1.277 2020/07/19 22:22:01 rillig Exp $";
+static char rcsid[] = "$NetBSD: var.c,v 1.278 2020/07/20 14:50:41 rillig Exp $";
 #else
 #include <sys/cdefs.h>
 #ifndef lint
 #if 0
 static char sccsid[] = "@(#)var.c      8.3 (Berkeley) 3/19/94";
 #else
-__RCSID("$NetBSD: var.c,v 1.277 2020/07/19 22:22:01 rillig Exp $");
+__RCSID("$NetBSD: var.c,v 1.278 2020/07/20 14:50:41 rillig Exp $");
 #endif
 #endif /* not lint */
 #endif
@@ -1106,103 +1106,110 @@
 }
 
 
+/* SepBuf is a string being built from "words", interleaved with separators. */
+typedef struct {
+    Buffer buf;
+    Boolean needSep;
+    char sep;
+} SepBuf;
+
+static void
+SepBuf_Init(SepBuf *buf, char sep)
+{
+    Buf_Init(&buf->buf, 32 /* bytes */);
+    buf->needSep = FALSE;
+    buf->sep = sep;
+}
+
+static void
+SepBuf_Sep(SepBuf *buf)
+{
+    buf->needSep = TRUE;
+}
+
+static void
+SepBuf_AddBytes(SepBuf *buf, const void *mem, size_t mem_size)
+{
+    if (mem_size == 0)
+       return;
+    if (buf->needSep && buf->sep != '\0') {
+       Buf_AddByte(&buf->buf, buf->sep);
+       buf->needSep = FALSE;
+    }
+    Buf_AddBytes(&buf->buf, mem_size, mem);
+}
+
+static char *
+SepBuf_Destroy(SepBuf *buf, Boolean free_buf)
+{
+    return Buf_Destroy(&buf->buf, free_buf);
+}
+
+
 /* This callback for VarModify gets a single word from an expression and
  * typically adds a modification of this word to the buffer. It may also do
- * nothing or add several words.
- *
- * If addSpaces is TRUE, it must add a space before adding anything else to
- * the buffer.
- *
- * It returns the addSpace value for the next call of this callback. Typical
- * return values are the current addSpaces or TRUE. */
-typedef Boolean (*VarModifyCallback)(GNode *ctxt, Var_Parse_State *vpstate,
-    const char *word, Boolean addSpace, Buffer *buf, void *data);
+ * nothing or add several words. */
+typedef void (*VarModifyCallback)(GNode *ctxt, const char *word, SepBuf *buf,
+                                 void *data);
 
 
 /* Callback function for VarModify to implement the :H modifier.
  * Add the dirname of the given word to the buffer. */
-static Boolean
-VarHead(GNode *ctx MAKE_ATTR_UNUSED, Var_Parse_State *vpstate,
-       const char *word, Boolean addSpace, Buffer *buf,
+static void
+VarHead(GNode *ctx MAKE_ATTR_UNUSED, const char *word, SepBuf *buf,
        void *dummy MAKE_ATTR_UNUSED)
 {
     const char *slash = strrchr(word, '/');
-
-    if (addSpace && vpstate->varSpace)
-       Buf_AddByte(buf, vpstate->varSpace);
     if (slash != NULL)
-       Buf_AddBytes(buf, slash - word, word);
+       SepBuf_AddBytes(buf, word, slash - word);
     else
-       Buf_AddByte(buf, '.');
-
-    return TRUE;
+       SepBuf_AddBytes(buf, ".", 1);
 }
 
 /* Callback function for VarModify to implement the :T modifier.
  * Add the basename of the given word to the buffer. */
-static Boolean
-VarTail(GNode *ctx MAKE_ATTR_UNUSED, Var_Parse_State *vpstate,
-       const char *word, Boolean addSpace, Buffer *buf,
+static void
+VarTail(GNode *ctx MAKE_ATTR_UNUSED, const char *word, SepBuf *buf,
        void *dummy MAKE_ATTR_UNUSED)
 {
     const char *slash = strrchr(word, '/');
     const char *base = slash != NULL ? slash + 1 : word;
-
-    if (addSpace && vpstate->varSpace != '\0')
-       Buf_AddByte(buf, vpstate->varSpace);
-    Buf_AddBytes(buf, strlen(base), base);
-    return TRUE;
+    SepBuf_AddBytes(buf, base, strlen(base));
 }
 
 /* Callback function for VarModify to implement the :E modifier.
  * Add the filename suffix of the given word to the buffer, if it exists. */
-static Boolean
-VarSuffix(GNode *ctx MAKE_ATTR_UNUSED, Var_Parse_State *vpstate,
-         const char *word, Boolean addSpace, Buffer *buf,
+static void
+VarSuffix(GNode *ctx MAKE_ATTR_UNUSED, const char *word, SepBuf *buf,
          void *dummy MAKE_ATTR_UNUSED)
 {
     const char *dot = strrchr(word, '.');
-    if (dot == NULL)
-       return addSpace;
-
-    if (addSpace && vpstate->varSpace != '\0')
-       Buf_AddByte(buf, vpstate->varSpace);
-    Buf_AddBytes(buf, strlen(dot + 1), dot + 1);
-    return TRUE;
+    if (dot != NULL)
+       SepBuf_AddBytes(buf, dot + 1, strlen(dot + 1));
 }
 
 /* Callback function for VarModify to implement the :R modifier.
  * Add the filename basename of the given word to the buffer. */
-static Boolean
-VarRoot(GNode *ctx MAKE_ATTR_UNUSED, Var_Parse_State *vpstate,
-       const char *word, Boolean addSpace, Buffer *buf,
+static void
+VarRoot(GNode *ctx MAKE_ATTR_UNUSED, const char *word, SepBuf *buf,
        void *dummy MAKE_ATTR_UNUSED)
 {
     char *dot = strrchr(word, '.');
     size_t len = dot != NULL ? (size_t)(dot - word) : strlen(word);
-
-    if (addSpace && vpstate->varSpace != '\0')
-       Buf_AddByte(buf, vpstate->varSpace);
-    Buf_AddBytes(buf, len, word);
-    return TRUE;
+    SepBuf_AddBytes(buf, word, len);
 }
 
 /* Callback function for VarModify to implement the :M modifier.
  * Place the word in the buffer if it matches the given pattern. */
-static Boolean
-VarMatch(GNode *ctx MAKE_ATTR_UNUSED, Var_Parse_State *vpstate,
-        const char *word, Boolean addSpace, Buffer *buf,
+static void
+VarMatch(GNode *ctx MAKE_ATTR_UNUSED, const char *word, SepBuf *buf,
         void *data)
 {
     const char *pattern = data;
     if (DEBUG(VAR))
        fprintf(debug_file, "VarMatch [%s] [%s]\n", word, pattern);
-    if (!Str_Match(word, pattern))
-       return addSpace;
-    if (addSpace && vpstate->varSpace != '\0')
-       Buf_AddByte(buf, vpstate->varSpace);
-    Buf_AddBytes(buf, strlen(word), word);
-    return TRUE;
+    if (Str_Match(word, pattern))
+       SepBuf_AddBytes(buf, word, strlen(word));
 }
 
 #ifdef SYSVVARSUB
@@ -1219,10 +1226,6 @@
  * Results:
  *     Returns the beginning position of a match or null. The number
  *     of characters matched is returned in len.
- *
- * Side Effects:
- *     None
- *
  *-----------------------------------------------------------------------
  */
 static const char *
@@ -1283,28 +1286,27 @@
  *
  * Side Effects:
  *     Places result on buf
- *
  *-----------------------------------------------------------------------
  */
 static void
-Str_SYSVSubst(Buffer *buf, const char *pat, const char *src, size_t len,
-    Boolean lhsHasPercent)
+Str_SYSVSubst(SepBuf *buf, const char *pat, const char *src, size_t len,
+             Boolean lhsHasPercent)
 {
     const char *m;
 
     if ((m = strchr(pat, '%')) != NULL && lhsHasPercent) {
        /* Copy the prefix */
-       Buf_AddBytes(buf, m - pat, pat);
+       SepBuf_AddBytes(buf, pat, m - pat);
        /* skip the % */
        pat = m + 1;
     }
     if (m != NULL || !lhsHasPercent) {
        /* Copy the pattern */
-       Buf_AddBytes(buf, len, src);
+       SepBuf_AddBytes(buf, src, len);
     }
 
     /* append the rest */
-    Buf_AddBytes(buf, strlen(pat), pat);
+    SepBuf_AddBytes(buf, pat, strlen(pat));
 }
 
 
@@ -1314,16 +1316,11 @@
 } VarSYSVSubstArgs;
 
 /* Callback function for VarModify to implement the :%.from=%.to modifier. */
-static Boolean
-VarSYSVSubst(GNode *ctx, Var_Parse_State *vpstate,