- give a specific value to OP_ERROR that doesn't occur in nature

- define OP_ZERO as zero, to make some function calls obvious
- split ParseDoOp into two functions: ParseDoOp that only deals with : :: !
and ParseDoSpecial that only deals with special nodes. This simplifies both
functions accordingly
- always initialize special_op

okay millert@

4 files changed, 74 insertions, 55 deletions

diff --git a/usr.bin/make/garray.h b/usr.bin/make/garray.h
index 13bb4840243..2dc60b5fd1d 100644
--- a/usr.bin/make/garray.h
+++ b/usr.bin/make/garray.h
@@ -1,7 +1,7 @@
 #ifndef GARRAY_H
 #define GARRAY_H
 
-/* $OpenBSD: garray.h,v 1.9 2014/05/18 08:08:50 espie Exp $ */
+/* $OpenBSD: garray.h,v 1.10 2019/12/22 16:53:40 espie Exp $ */
 /* Growable array implementation */
 
 /*
@@ -79,7 +79,7 @@ do {						\
 	unsigned int i;				\
 	for (i = 0; i < (l)->n; i++)		\
 		if ((func)((l)->a[i], (v)) == 0)\
-		    break;			\
+		    	break;			\
 } while (0)
 
 #define Array_FindP(l, func, v)				\
@@ -87,7 +87,7 @@ do {							\
 	unsigned int i;					\
 	for (i = 0; i < (l)->n; i++)			\
 		if ((func)(&((l)->a[i]), (v)) == 0)	\
-		    break;				\
+		    	break;				\
 } while (0)
 
 #define Array_ForEach(l, func, v)		\
diff --git a/usr.bin/make/gnode.h b/usr.bin/make/gnode.h
index 6feea19d570..b8a33036fbd 100644
--- a/usr.bin/make/gnode.h
+++ b/usr.bin/make/gnode.h
@@ -1,6 +1,6 @@
 #ifndef GNODE_H
 #define GNODE_H
-/*	$OpenBSD: gnode.h,v 1.33 2019/12/21 15:31:54 espie Exp $ */
+/*	$OpenBSD: gnode.h,v 1.34 2019/12/22 16:53:40 espie Exp $ */
 
 /*
  * Copyright (c) 2001 Marc Espie.
@@ -172,12 +172,13 @@ struct command
  * the lefthand side of an operator, though it may have been on the
  * righthand side...
  */
+#define OP_ZERO		0x00000000  /* No dependency operator seen so far */
 #define OP_DEPENDS	0x00000001  /* Execution of commands depends on
 				     * kids (:) */
 #define OP_FORCE	0x00000002  /* Always execute commands (!) */
 #define OP_DOUBLEDEP	0x00000004  /* Execution of commands depends on kids
 				     * per line (::) */
-#define OP_ERROR	0x00000000
+#define OP_ERROR	0x00000007
 #define OP_OPMASK	(OP_DEPENDS|OP_FORCE|OP_DOUBLEDEP)
 
 #define OP_OPTIONAL	0x00000008  /* Don't care if the target doesn't
@@ -227,7 +228,7 @@ struct command
  * OP_NOP will return true if the node with the given type was not the
  * object of a dependency operator
  */
-#define OP_NOP(t)	(((t) & OP_OPMASK) == 0x00000000)
+#define OP_NOP(t)	(((t) & OP_OPMASK) == OP_ZERO)
 
 #define OP_NOTARGET (OP_NOTMAIN|OP_USE|OP_EXEC|OP_TRANSFORM)
 
diff --git a/usr.bin/make/parse.c b/usr.bin/make/parse.c
index 287cf6bae20..e8276b12f44 100644
--- a/usr.bin/make/parse.c
+++ b/usr.bin/make/parse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.c,v 1.122 2019/12/21 15:29:25 espie Exp $	*/
+/*	$OpenBSD: parse.c,v 1.123 2019/12/22 16:53:40 espie Exp $	*/
 /*	$NetBSD: parse.c,v 1.29 1997/03/10 21:20:04 christos Exp $	*/
 
 /*
@@ -131,6 +131,7 @@ static GNode	*predecessor;
 
 static void ParseLinkSrc(GNode *, GNode *);
 static int ParseDoOp(GNode **, unsigned int);
+static void ParseDoSpecial(GNode *, unsigned int);
 static int ParseAddDep(GNode *, GNode *);
 static void ParseDoSrc(struct growableArray *, struct growableArray *, int,
     const char *, const char *);
@@ -282,65 +283,81 @@ operator_string(int op)
  *	Apply the parsed operator to the given target node. Used in a
  *	Array_Find call by ParseDoDependency once all targets have
  *	been found and their operator parsed. If the previous and new
- *	operators are incompatible, a major error is taken.
+ *	operators are incompatible, a major error is taken, and the find
+ *	stops early
  *
  * Side Effects:
- *	The type field of the node is altered to reflect any new bits in
- *	the op.
+ *	The node gets the right dependency operator.
+ *	Cohorts may be created for double dep.
  *---------------------------------------------------------------------
  */
 static int
 ParseDoOp(GNode **gnp, unsigned int op)
 {
 	GNode *gn = *gnp;
-	/*
-	 * If the dependency mask of the operator and the node don't match and
-	 * the node has actually had an operator applied to it before, and the
-	 * operator actually has some dependency information in it, complain.
-	 */
-	if (((op & OP_OPMASK) != (gn->type & OP_OPMASK)) &&
-	    !OP_NOP(gn->type) && !OP_NOP(op)) {
-		Parse_Error(PARSE_FATAL, 
-		    "Inconsistent dependency operator for target %s\n"
-		    "\t(was %s%s, now %s%s)",
-		    gn->name, gn->name, operator_string(gn->type), 
-		    gn->name, operator_string(op));
-		return 0;
-	}
 
-	if (op == OP_DOUBLEDEP && ((gn->type & OP_OPMASK) == OP_DOUBLEDEP)) {
-		/* If the node was the object of a :: operator, we need to
-		 * create a new instance of it for the children and commands on
-		 * this dependency line. The new instance is placed on the
-		 * 'cohorts' list of the initial one (note the initial one is
-		 * not on its own cohorts list) and the new instance is linked
-		 * to all parents of the initial instance.  */
-		GNode *cohort;
-		LstNode ln;
-
-		cohort = Targ_NewGN(gn->name);
-		/* Duplicate links to parents so graph traversal is simple.
-		 * Perhaps some type bits should be duplicated?
-		 *
-		 * Make the cohort invisible as well to avoid duplicating it
-		 * into other variables. True, parents of this target won't
-		 * tend to do anything with their local variables, but better
-		 * safe than sorry.  */
-		for (ln = Lst_First(&gn->parents); ln != NULL; ln = Lst_Adv(ln))
-			ParseLinkSrc(Lst_Datum(ln), cohort);
-		cohort->type = OP_DOUBLEDEP|OP_INVISIBLE;
-		Lst_AtEnd(&gn->cohorts, cohort);
-
-		/* Replace the node in the targets list with the new copy */
-		*gnp = cohort;
-		gn = cohort;
+	assert(op == (op & OP_OPMASK));
+
+	/* if the node didn't already appear on the left hand side (no known
+	 * dependency operator), we don't need to do much. */
+	if (!OP_NOP(gn->type)) {
+		/*
+		 * If the dependency mask of the operator and the node don't
+		 * match and the node has actually had an operator applied to
+		 * it before, and the operator actually has some dependency
+		 * information in it, complain.  */
+		if (op != (gn->type & OP_OPMASK)) {
+			Parse_Error(PARSE_FATAL, 
+			    "Inconsistent dependency operator for target %s\n"
+			    "\t(was %s%s, now %s%s)",
+			    gn->name, gn->name, operator_string(gn->type), 
+			    gn->name, operator_string(op));
+			return 0;
+		}
+
+		if (op == OP_DOUBLEDEP) {
+			/* If the node was the object of a :: operator, we need
+			 * to create a new instance of it for the children and
+			 * commands on this dependency line. The new instance
+			 * is placed on the 'cohorts' list of the initial one
+			 * (note the initial one is not on its own cohorts
+			 * list) and the new instance is linked to all parents
+			 * of the initial instance.  */
+			GNode *cohort;
+			LstNode ln;
+
+			cohort = Targ_NewGN(gn->name);
+			/* Duplicate links to parents so graph traversal is
+			 * simple.  Perhaps some type bits should be
+			 * duplicated?
+			 *
+			 * Make the cohort invisible as well to avoid
+			 * duplicating it into other variables. True, parents
+			 * of this target won't tend to do anything with their
+			 * local variables, but better safe than sorry.  */
+			for (ln = Lst_First(&gn->parents); ln != NULL; 
+			    ln = Lst_Adv(ln))
+				ParseLinkSrc(Lst_Datum(ln), cohort);
+			cohort->type = OP_DOUBLEDEP|OP_INVISIBLE;
+			Lst_AtEnd(&gn->cohorts, cohort);
+
+			/* Replace the node in the targets list with the new
+			 * copy */
+			*gnp = cohort;
+			gn = cohort;
+		}
 	}
-	/* We don't want to nuke any previous flags (whatever they were) so we
-	 * just OR the new operator into the old.  */
+	/* Preserve possible special flags already applied to the operator */
 	gn->type |= op;
 	return 1;
 }
 
+static void
+ParseDoSpecial(GNode *gn, unsigned int special_op)
+{
+	gn->type |= special_op;
+}
+
 /*-
  *---------------------------------------------------------------------
  * ParseAddDep	--
@@ -404,7 +421,7 @@ ParseDoSrc(
 	GNode *gn = Targ_FindNodei(src, esrc, TARG_CREATE);
 	if ((gn->special & SPECIAL_SOURCE) != 0) {
 		if (gn->special_op) {
-			Array_FindP(targets, ParseDoOp, gn->special_op);
+			Array_ForEach(targets, ParseDoSpecial, gn->special_op);
 			return;
 		} else {
 			assert((gn->special & SPECIAL_MASK) == SPECIAL_WAIT);
diff --git a/usr.bin/make/targ.c b/usr.bin/make/targ.c
index f88440013d0..3df06f7bf64 100644
--- a/usr.bin/make/targ.c
+++ b/usr.bin/make/targ.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: targ.c,v 1.81 2019/12/21 15:31:54 espie Exp $ */
+/*	$OpenBSD: targ.c,v 1.82 2019/12/22 16:53:40 espie Exp $ */
 /*	$NetBSD: targ.c,v 1.11 1997/02/20 16:51:50 christos Exp $	*/
 
 /*
@@ -150,8 +150,9 @@ Targ_NewGNi(const char *name, const char *ename)
 
 	gn = ohash_create_entry(&gnode_info, name, &ename);
 	gn->path = NULL;
-	gn->type = 0;
+	gn->type = OP_ZERO;
 	gn->special = SPECIAL_NONE;
+	gn->special_op = 0;
 	gn->children_left = 0;
 	gn->must_make = false;
 	gn->built_status = UNKNOWN;