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: replace enum bit-field with struct bit-fi...



details:   https://anonhg.NetBSD.org/src/rev/bf7c356c9439
branches:  trunk
changeset: 953656:bf7c356c9439
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon Mar 15 12:15:03 2021 +0000

description:
make: replace enum bit-field with struct bit-field for VarEvalFlags

This makes the code easier to read, especially in var.c.  It also makes
debugging sessions easier since some debuggers don't show enum
bit-fields symbolically as soon as more than one bit is set.

The code outside var.c is basically unchanged, except that instead of
passing the individual flags, there are 4 predefined evaluation modes.
These suffice for all practical use cases.  Only in the implementation
deep inside var.c, the value of the flags keepDollar and keepUndef
differs.

There is no way of passing the struct to EnumFlags_ToString, which means
the ToString function has to be spelled out explicitly.  This allows for
fine-tuning the representation in the debug log, to reduce the amount of
uppercae letters.

No functional change.

diffstat:

 usr.bin/make/arch.c                        |   17 +--
 usr.bin/make/cond.c                        |    8 +-
 usr.bin/make/meta.c                        |    4 +-
 usr.bin/make/nonints.h                     |   37 +++++--
 usr.bin/make/parse.c                       |   13 +-
 usr.bin/make/suff.c                        |    6 +-
 usr.bin/make/unit-tests/cond-func-empty.mk |   10 +-
 usr.bin/make/unit-tests/recursive.mk       |    7 +-
 usr.bin/make/unit-tests/varmod-defined.mk  |    4 +-
 usr.bin/make/unit-tests/varmod-ifelse.mk   |    4 +-
 usr.bin/make/unit-tests/varmod-loop.mk     |   20 ++--
 usr.bin/make/unit-tests/varparse-errors.mk |    4 +-
 usr.bin/make/var.c                         |  142 +++++++++++++++++-----------
 13 files changed, 158 insertions(+), 118 deletions(-)

diffs (truncated from 888 to 300 lines):

diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/arch.c
--- a/usr.bin/make/arch.c       Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/arch.c       Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: arch.c,v 1.197 2021/02/05 05:15:12 rillig Exp $        */
+/*     $NetBSD: arch.c,v 1.198 2021/03/15 12:15:03 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -126,7 +126,7 @@
 #include "config.h"
 
 /*     "@(#)arch.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: arch.c,v 1.197 2021/02/05 05:15:12 rillig Exp $");
+MAKE_RCSID("$NetBSD: arch.c,v 1.198 2021/03/15 12:15:03 rillig Exp $");
 
 typedef struct List ArchList;
 typedef struct ListNode ArchListNode;
@@ -207,7 +207,7 @@
 
                        /* XXX: is expanded twice: once here and once below */
                        (void)Var_Parse(&nested_p, scope,
-                                       VARE_WANTRES | VARE_UNDEFERR, &result);
+                           VARE_UNDEFERR, &result);
                        /* TODO: handle errors */
                        isError = result.str == var_Error;
                        FStr_Done(&result);
@@ -223,8 +223,7 @@
        *cp++ = '\0';
        if (expandLibName) {
                char *expanded;
-               (void)Var_Subst(libName.str, scope,
-                   VARE_WANTRES | VARE_UNDEFERR, &expanded);
+               (void)Var_Subst(libName.str, scope, VARE_UNDEFERR, &expanded);
                /* TODO: handle errors */
                libName = MFStr_InitOwn(expanded);
        }
@@ -250,8 +249,7 @@
                                const char *nested_p = cp;
 
                                (void)Var_Parse(&nested_p, scope,
-                                               VARE_WANTRES | VARE_UNDEFERR,
-                                               &result);
+                                   VARE_UNDEFERR, &result);
                                /* TODO: handle errors */
                                isError = result.str == var_Error;
                                FStr_Done(&result);
@@ -306,9 +304,8 @@
                        char *p;
                        char *unexpandedMemName = memName;
 
-                       (void)Var_Subst(memName, scope,
-                                       VARE_WANTRES | VARE_UNDEFERR,
-                                       &memName);
+                       (void)Var_Subst(memName, scope, VARE_UNDEFERR,
+                           &memName);
                        /* TODO: handle errors */
 
                        /*
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/cond.c
--- a/usr.bin/make/cond.c       Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/cond.c       Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cond.c,v 1.258 2021/03/15 11:41:07 rillig Exp $        */
+/*     $NetBSD: cond.c,v 1.259 2021/03/15 12:15:03 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -95,7 +95,7 @@
 #include "dir.h"
 
 /*     "@(#)cond.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: cond.c,v 1.258 2021/03/15 11:41:07 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.259 2021/03/15 12:15:03 rillig Exp $");
 
 /*
  * The parsing of conditional expressions is based on this grammar:
@@ -265,7 +265,7 @@
                         * error, though perhaps we should.
                         */
                        VarEvalFlags eflags = doEval
-                           ? VARE_WANTRES | VARE_UNDEFERR
+                           ? VARE_UNDEFERR
                            : VARE_PARSE_ONLY;
                        FStr nestedVal;
                        (void)Var_Parse(&p, SCOPE_CMDLINE, eflags, &nestedVal);
@@ -420,7 +420,7 @@
        VarParseResult parseResult;
 
        /* if we are in quotes, an undefined variable is ok */
-       eflags = doEval && !quoted ? VARE_WANTRES | VARE_UNDEFERR
+       eflags = doEval && !quoted ? VARE_UNDEFERR
            : doEval ? VARE_WANTRES
            : VARE_PARSE_ONLY;
 
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/meta.c
--- a/usr.bin/make/meta.c       Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/meta.c       Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*      $NetBSD: meta.c,v 1.178 2021/02/22 23:21:33 rillig Exp $ */
+/*      $NetBSD: meta.c,v 1.179 2021/03/15 12:15:03 rillig Exp $ */
 
 /*
  * Implement 'meta' mode.
@@ -1516,7 +1516,7 @@
                        DEBUG2(META, "%s: %d: cannot compare command using .OODATE\n",
                               fname, lineno);
                    }
-                   (void)Var_Subst(cmd, gn, VARE_WANTRES|VARE_UNDEFERR, &cmd);
+                   (void)Var_Subst(cmd, gn, VARE_UNDEFERR, &cmd);
                    /* TODO: handle errors */
 
                    if ((cp = strchr(cmd, '\n')) != NULL) {
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/nonints.h
--- a/usr.bin/make/nonints.h    Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/nonints.h    Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nonints.h,v 1.204 2021/03/15 11:41:07 rillig Exp $     */
+/*     $NetBSD: nonints.h,v 1.205 2021/03/15 12:15:03 rillig Exp $     */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -292,22 +292,25 @@
 void Var_Init(void);
 void Var_End(void);
 
-typedef enum VarEvalFlags {
-       VARE_PARSE_ONLY         = 0,
+typedef struct VarEvalFlags {
 
        /*
         * Expand and evaluate variables during parsing.
         *
+        * Without this flag, the expression is only parsed, without
+        * evaluating any part of it.
+        *
         * TODO: Document what Var_Parse and Var_Subst return when this flag
-        * is not set.
+        *  is not set.  As of 2021-03-15, they return unspecified,
+        *  inconsistent results.
         */
-       VARE_WANTRES            = 1 << 0,
+       Boolean wantRes: 1;
 
        /*
         * Treat undefined variables as errors.
-        * Must only be used in combination with VARE_WANTRES.
+        * Must only be used in combination with wantRes.
         */
-       VARE_UNDEFERR           = 1 << 1,
+       Boolean undefErr: 1;
 
        /*
         * Keep '$$' as '$$' instead of reducing it to a single '$'.
@@ -317,7 +320,7 @@
         * expanding '$$file' to '$file' in the first assignment and
         * interpreting it as '${f}' followed by 'ile' in the next assignment.
         */
-       VARE_KEEP_DOLLAR        = 1 << 2,
+       Boolean keepDollar: 1;
 
        /*
         * Keep undefined variables as-is instead of expanding them to an
@@ -330,9 +333,22 @@
         *      # way) is still undefined, the updated CFLAGS becomes
         *      # "-I.. $(.INCLUDES)".
         */
-       VARE_KEEP_UNDEF         = 1 << 3
+       Boolean keepUndef: 1;
+
+       /*
+        * Without this padding, GCC 9.3.0 on NetBSD 9.99.80 generates larger
+        * code than necessary (1.2 kB), masking out the unused bits from the
+        * int (since that is the default representation of Boolean in make),
+        * even for initializers consisting entirely of constants.
+        */
+       Boolean : 1;
 } VarEvalFlags;
 
+#define VARE_PARSE_ONLY        (VarEvalFlags) { FALSE, FALSE, FALSE, FALSE }
+#define VARE_WANTRES   (VarEvalFlags) { TRUE, FALSE, FALSE, FALSE }
+#define VARE_UNDEFERR  (VarEvalFlags) { TRUE, TRUE, FALSE, FALSE }
+#define VARE_KEEP_DOLLAR_UNDEF (VarEvalFlags) { TRUE, FALSE, TRUE, TRUE }
+
 typedef enum VarSetFlags {
        VAR_SET_NONE            = 0,
 
@@ -361,7 +377,8 @@
         * Some callers handle this case differently, so return this
         * information to them, for now.
         *
-        * TODO: Replace this with a new flag VARE_KEEP_UNDEFINED.
+        * TODO: Instead of having this special return value, rather ensure
+        *  that VarEvalFlags.keepUndef is processed properly.
         */
        VPR_UNDEF
 
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/parse.c      Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.551 2021/03/15 11:41:07 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.552 2021/03/15 12:15:03 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -109,7 +109,7 @@
 #include "pathnames.h"
 
 /*     "@(#)parse.c    8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.551 2021/03/15 11:41:07 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.552 2021/03/15 12:15:03 rillig Exp $");
 
 /* types and constants */
 
@@ -1898,8 +1898,7 @@
        if (!Var_ExistsExpand(scope, name))
                Var_SetExpand(scope, name, "");
 
-       (void)Var_Subst(uvalue, scope,
-           VARE_WANTRES | VARE_KEEP_DOLLAR | VARE_KEEP_UNDEF, &evalue);
+       (void)Var_Subst(uvalue, scope, VARE_KEEP_DOLLAR_UNDEF, &evalue);
        /* TODO: handle errors */
 
        Var_SetExpand(scope, name, evalue);
@@ -1918,8 +1917,8 @@
        cmd = FStr_InitRefer(uvalue);
        if (strchr(cmd.str, '$') != NULL) {
                char *expanded;
-               (void)Var_Subst(cmd.str, SCOPE_CMDLINE,
-                   VARE_WANTRES | VARE_UNDEFERR, &expanded);
+               (void)Var_Subst(cmd.str, SCOPE_CMDLINE, VARE_UNDEFERR,
+                   &expanded);
                /* TODO: handle errors */
                cmd = FStr_InitOwn(expanded);
        }
@@ -3131,7 +3130,7 @@
         * Var_Parse does not print any parse errors in such a case.
         * It simply returns the special empty string var_Error,
         * which cannot be detected in the result of Var_Subst. */
-       eflags = opts.strict ? VARE_WANTRES : VARE_WANTRES | VARE_UNDEFERR;
+       eflags = opts.strict ? VARE_WANTRES : VARE_UNDEFERR;
        (void)Var_Subst(line, SCOPE_CMDLINE, eflags, &expanded_line);
        /* TODO: handle errors */
 
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/suff.c
--- a/usr.bin/make/suff.c       Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/suff.c       Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: suff.c,v 1.347 2021/03/15 11:41:07 rillig Exp $        */
+/*     $NetBSD: suff.c,v 1.348 2021/03/15 12:15:03 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -114,7 +114,7 @@
 #include "dir.h"
 
 /*     "@(#)suff.c     8.4 (Berkeley) 3/21/94" */
-MAKE_RCSID("$NetBSD: suff.c,v 1.347 2021/03/15 11:41:07 rillig Exp $");
+MAKE_RCSID("$NetBSD: suff.c,v 1.348 2021/03/15 12:15:03 rillig Exp $");
 
 typedef List SuffixList;
 typedef ListNode SuffixListNode;
@@ -1380,7 +1380,7 @@
        }
 
        DEBUG1(SUFF, "Expanding \"%s\"...", cgn->name);
-       (void)Var_Subst(cgn->name, pgn, VARE_WANTRES | VARE_UNDEFERR, &cp);
+       (void)Var_Subst(cgn->name, pgn, VARE_UNDEFERR, &cp);
        /* TODO: handle errors */
 
        {
diff -r 5f21f9890c6c -r bf7c356c9439 usr.bin/make/unit-tests/cond-func-empty.mk
--- a/usr.bin/make/unit-tests/cond-func-empty.mk        Mon Mar 15 11:41:07 2021 +0000
+++ b/usr.bin/make/unit-tests/cond-func-empty.mk        Mon Mar 15 12:15:03 2021 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: cond-func-empty.mk,v 1.12 2021/02/22 20:38:55 rillig Exp $
+# $NetBSD: cond-func-empty.mk,v 1.13 2021/03/15 12:15:03 rillig Exp $
 #
 # Tests for the empty() function in .if conditions, which tests a variable
 # expression for emptiness.
@@ -93,8 +93,8 @@
 # neither leading nor trailing spaces are trimmed in the argument of the
 # function.  If the spaces were trimmed, the variable name would be "" and
 # that variable is indeed undefined.  Since ParseEmptyArg calls Var_Parse
-# without VARE_UNDEFERR, the value of the undefined variable is returned as
-# an empty string.
+# without VarEvalFlags.undefErr, the value of the undefined variable is
+# returned as an empty string.
 ${:U }=        space
 .if empty( )
 .  error
@@ -168,14 +168,14 @@
 # parsing it, this unrealistic variable name should have done no harm.
 #
 # The variable expression was expanded though, and this was wrong.  The
-# expansion was done without the VARE_WANTRES flag (called VARF_WANTRES back
+# expansion was done without VarEvalFlags.wantRes (called VARF_WANTRES back
 # then) though.  This had the effect that the ${:U1} from the value of VARNAME
 # expanded to an empty string.  This in turn created the seemingly recursive



Home | Main Index | Thread Index | Old Index