[src/trunk]: src/usr.bin/make make(1): use bitset for IfState

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

details:   https://anonhg.NetBSD.org/src/rev/42baced5a48f
branches:  trunk
changeset: 956983:42baced5a48f
user:      rillig <rillig%NetBSD.org@localhost>
date:      Fri Nov 13 07:52:03 2020 +0000

description:
make(1): use bitset for IfState

Previously, the individual IfStates contained redundant information,
which was apparent in the documentation.  This led to expressions like
(state > ELSE_ACTIVE) that are hard to read since the reader has to look
up the order of the enum.

To avoid this, the state of an '.if' block is now encoded using a bitset,
encoding the properties of each state directly.  This replaces the
previous (state > ELSE_ACTIVE) with !(state & IFS_ACTIVE), which is
easier to understand.

No change in behavior.

diffstat:

 usr.bin/make/cond.c |  69 +++++++++++++++++++++++-----------------------------
 1 files changed, 31 insertions(+), 38 deletions(-)

diffs (156 lines):

diff -r a83371d419c0 -r 42baced5a48f usr.bin/make/cond.c
--- a/usr.bin/make/cond.c       Fri Nov 13 07:35:27 2020 +0000
+++ b/usr.bin/make/cond.c       Fri Nov 13 07:52:03 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cond.c,v 1.212 2020/11/13 07:35:27 rillig Exp $        */
+/*     $NetBSD: cond.c,v 1.213 2020/11/13 07:52:03 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -94,7 +94,7 @@
 #include "dir.h"
 
 /*     "@(#)cond.c     8.2 (Berkeley) 1/2/94"  */
-MAKE_RCSID("$NetBSD: cond.c,v 1.212 2020/11/13 07:35:27 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.213 2020/11/13 07:52:03 rillig Exp $");
 
 /*
  * The parsing of conditional expressions is based on this grammar:
@@ -1092,26 +1092,18 @@
 {
     typedef enum IfState {
 
-       /* The previous <cond> evaluated to TRUE.  The lines following this
-        * condition are interpreted. */
-       IF_ACTIVE,
-
-       /* The previous '.else' evaluated to TRUE.  The lines following this
-        * condition are interpreted.  The only difference to IF_ACTIVE is
-        * that no other '.else' may follow. */
-       ELSE_ACTIVE,
+       /* None of the previous <cond> evaluated to TRUE. */
+       IFS_INITIAL     = 0,
 
-       /* All of the previous <cond> evaluated to FALSE.  Still searching
-        * for an '.elif' or an 'else' that evaluates to TRUE. */
-       SEARCH_FOR_ELIF,
+       /* The previous <cond> evaluated to TRUE.
+        * The lines following this condition are interpreted. */
+       IFS_ACTIVE      = 1 << 0,
 
-       /* One of the previous <cond> evaluated to TRUE.  There was no '.else'
-        * yet. */
-       SKIP_TO_ELSE,
+       /* The previous directive was an '.else'. */
+       IFS_SEEN_ELSE   = 1 << 1,
 
-       /* One of the previous <cond> evaluated to TRUE, and '.else' was
-        * already seen.  No other '.else' may follow. */
-       SKIP_TO_ENDIF
+       /* One of the previous <cond> evaluated to TRUE. */
+       IFS_WAS_ACTIVE  = 1 << 2
 
     } IfState;
 
@@ -1126,7 +1118,7 @@
 
     if (cond_states == NULL) {
        cond_states = bmake_malloc(cond_states_cap * sizeof *cond_states);
-       cond_states[0] = IF_ACTIVE;
+       cond_states[0] = IFS_ACTIVE;
     }
 
     p++;               /* skip the leading '.' */
@@ -1136,8 +1128,8 @@
     if (p[0] == 'e') {
        if (p[1] != 'l') {
            if (!is_token(p + 1, "ndif", 4)) {
-               /* Unknown directive.  It might still be a transformation
-                * rule like '.elisp.scm', therefore no error message here. */
+               /* Unknown directive.  It might still be a transformation
+                * rule like '.elisp.scm', therefore no error message here. */
                return COND_INVALID;
            }
 
@@ -1151,7 +1143,7 @@
 
            /* Return state for previous conditional */
            cond_depth--;
-           return cond_states[cond_depth] <= ELSE_ACTIVE
+           return cond_states[cond_depth] & IFS_ACTIVE
                   ? COND_PARSE : COND_SKIP;
        }
 
@@ -1167,15 +1159,16 @@
            }
 
            state = cond_states[cond_depth];
-           if (state == SEARCH_FOR_ELIF) {
-               state = ELSE_ACTIVE;
+           if (state == IFS_INITIAL) {
+               state = IFS_ACTIVE | IFS_SEEN_ELSE;
            } else {
-               if (state == ELSE_ACTIVE || state == SKIP_TO_ENDIF)
+               if (state & IFS_SEEN_ELSE)
                    Parse_Error(PARSE_WARNING, "extra else");
-               state = SKIP_TO_ENDIF;
+               state = IFS_WAS_ACTIVE | IFS_SEEN_ELSE;
            }
            cond_states[cond_depth] = state;
-           return state <= ELSE_ACTIVE ? COND_PARSE : COND_SKIP;
+
+           return state & IFS_ACTIVE ? COND_PARSE : COND_SKIP;
        }
        /* Assume for now it is an elif */
        isElif = TRUE;
@@ -1215,14 +1208,13 @@
            return COND_PARSE;
        }
        state = cond_states[cond_depth];
-       if (state == SKIP_TO_ENDIF || state == ELSE_ACTIVE) {
+       if (state & IFS_SEEN_ELSE) {
            Parse_Error(PARSE_WARNING, "extra elif");
-           cond_states[cond_depth] = SKIP_TO_ENDIF;
+           cond_states[cond_depth] = IFS_WAS_ACTIVE | IFS_SEEN_ELSE;
            return COND_SKIP;
        }
-       if (state != SEARCH_FOR_ELIF) {
-           /* Either just finished the 'true' block, or already SKIP_TO_ELSE */
-           cond_states[cond_depth] = SKIP_TO_ELSE;
+       if (state != IFS_INITIAL) {
+           cond_states[cond_depth] = IFS_WAS_ACTIVE;
            return COND_SKIP;
        }
     } else {
@@ -1239,9 +1231,9 @@
        }
        state = cond_states[cond_depth];
        cond_depth++;
-       if (state > ELSE_ACTIVE) {
+       if (!(state & IFS_ACTIVE)) {
            /* If we aren't parsing the data, treat as always false */
-           cond_states[cond_depth] = SKIP_TO_ELSE;
+           cond_states[cond_depth] = IFS_WAS_ACTIVE;
            return COND_SKIP;
        }
     }
@@ -1250,15 +1242,16 @@
     if (CondEvalExpression(ifp, p, &value, TRUE, TRUE) == COND_INVALID) {
        /* Syntax error in conditional, error message already output. */
        /* Skip everything to matching .endif */
-       cond_states[cond_depth] = SKIP_TO_ELSE;
+       /* XXX: An extra '.else' is not detected in this case. */
+       cond_states[cond_depth] = IFS_WAS_ACTIVE;
        return COND_SKIP;
     }
 
     if (!value) {
-       cond_states[cond_depth] = SEARCH_FOR_ELIF;
+       cond_states[cond_depth] = IFS_INITIAL;
        return COND_SKIP;
     }
-    cond_states[cond_depth] = IF_ACTIVE;
+    cond_states[cond_depth] = IFS_ACTIVE;
     return COND_PARSE;
 }