>Number:         9899
>Category:       bin
>Synopsis:       multi-variable .for constructs in make
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Apr 16 07:04:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     David A. Holland
>Release:        NetBSD-current 20000416
>Organization:

>Environment:

System: NetBSD merlin 1.4X NetBSD 1.4X (MERLIN) #30: Sun Apr 16 08:04:29 EDT 2000 dholland@merlin:/usr/src/sys/arch/i386/compile/MERLIN i386


>Description:

	The enclosed patch adds the ability to have .for loops in make that
	have more than one iteration variable. That is, if you do

		all:
		.for X Y in 1 2 3 4
			@echo $X $Y
		.endfor

	and run it, it'll print

		1 2
		3 4

	This has obvious uses for <bsd.links.mk> and the like. (Note that
	using the feature in those should wait until after there's been a
	release, so as to avoid breaking the upgrade path; so this should
	go in, if it's going to, before any 1.5 freeze.)

	The implementation has a couple of ugly bits, but correcting them
	requires major surgery to the variable handling code, which I 
	thought should be undertaken separately.

	The change should be completely backwards compatible, except for
	the highly unlikely case that someone tried to use "in" as an
	iteration variable.

	In any event, the patched make has no trouble building the world. 

	I have updated the man page. I have not updated PSD.doc, because
	it appears to be sufficiently out of date that it doesn't even 
	mention for loops.

>How-To-Repeat:

	N/A.

>Fix:

Index: usr.bin/make/for.c
===================================================================
RCS file: /cvsroot/basesrc/usr.bin/make/for.c,v
retrieving revision 1.6
diff -u -3 -r1.6 for.c
--- for.c	1997/09/28 03:31:03	1.6
+++ for.c	2000/04/16 13:13:00
@@ -57,6 +57,7 @@
  */
 
 #include    <ctype.h>
+#include    <assert.h>
 #include    "make.h"
 #include    "hash.h"
 #include    "dir.h"
@@ -74,27 +75,59 @@
  * and the .endfor statements, accumulating all the statements between
  * the initial .for loop and the matching .endfor;
  * then we evaluate the for loop for each variable in the varlist.
+ *
+ * Note that any nested fors are just passed through; they get handled
+ * recursively in For_Eval when we're expanding the enclosing for in
+ * For_Run.
  */
 
 static int  	  forLevel = 0;  	/* Nesting level	*/
-static char	 *forVar;		/* Iteration variable	*/
-static Buffer	  forBuf;		/* Commands in loop	*/
-static Lst	  forLst;		/* List of items	*/
 
 /*
  * State of a for loop.
  */
 typedef struct _For {
-    Buffer	  buf;			/* Unexpanded buffer	*/
-    char*	  var;			/* Index name		*/
-    Lst  	  lst;			/* List of variables	*/
+    Buffer	  buf;			/* Body of loop		*/
+    char	**vars;			/* Iteration variables	*/
+    int           nvars;		/* # of iteration vars	*/
+    Lst  	  lst;			/* List of items	*/
 } For;
 
-static int ForExec	__P((ClientData, ClientData));
+static For        accumFor;             /* Loop being accumulated */
 
+static void ForAddVar	__P((const char *, size_t));
 
 
 
+
+/*-
+ *-----------------------------------------------------------------------
+ * ForAddVar --
+ *	Add an iteration variable to the currently accumulating for.
+ *
+ * Results: none
+ * Side effects: no additional side effects.
+ *-----------------------------------------------------------------------
+ */
+static void
+ForAddVar(data, len)
+	const char *data;
+	size_t len;
+{
+	Buffer buf;
+	size_t varlen;
+
+	buf = Buf_Init(0);
+	Buf_AddBytes(buf, len, (Byte *) data);
+
+	accumFor.nvars++;
+	accumFor.vars = erealloc(accumFor.vars, accumFor.nvars*sizeof(char *));
+
+	accumFor.vars[accumFor.nvars-1] = (char *) Buf_GetAll(buf, &varlen);
+
+	Buf_Destroy(buf, FALSE);
+}
+
 /*-
  *-----------------------------------------------------------------------
  * For_Eval --
@@ -116,7 +149,7 @@
 For_Eval (line)
     char    	    *line;    /* Line to parse */
 {
-    char	    *ptr = line, *sub, *wrd;
+    char	    *ptr = line, *sub, *in, *wrd;
     int	    	    level;  	/* Level at which to report errors. */
 
     level = PARSE_FATAL;
@@ -144,48 +177,55 @@
 	    ptr++;
 
 	/*
-	 * Grab the variable
+	 * Find the "in".
 	 */
-	buf = Buf_Init(0);
-	for (wrd = ptr; *ptr && !isspace((unsigned char) *ptr); ptr++)
-	    continue;
-	Buf_AddBytes(buf, ptr - wrd, (Byte *) wrd);
-
-	forVar = (char *) Buf_GetAll(buf, &varlen);
-	if (varlen == 0) {
-	    Parse_Error (level, "missing variable in for");
+	for (in = ptr; *in; in++) {
+	    if (isspace(in[0]) && in[1]=='i' && in[2]=='n' &&
+		(in[3]==0 || isspace(in[3])))
+		    break;
+	}
+	if (*in==0) {
+	    Parse_Error(level, "missing `in' in for");
 	    return 0;
 	}
-	Buf_Destroy(buf, FALSE);
-
-	while (*ptr && isspace((unsigned char) *ptr))
-	    ptr++;
 
 	/*
-	 * Grab the `in'
+	 * Grab the variables.
 	 */
-	if (ptr[0] != 'i' || ptr[1] != 'n' ||
-	    !isspace((unsigned char) ptr[2])) {
-	    Parse_Error (level, "missing `in' in for");
-	    printf("%s\n", ptr);
+	accumFor.vars = NULL;
+
+	while (ptr < in) {
+	    wrd = ptr;
+	    while (*ptr && !isspace((unsigned char) *ptr))
+	        ptr++;
+	    ForAddVar(wrd, ptr - wrd);
+	    while (*ptr && isspace((unsigned char) *ptr))
+		ptr++;
+	}
+
+	if (accumFor.nvars==0) {
+	    Parse_Error(level, "no iteration variables in for");
 	    return 0;
 	}
-	ptr += 3;
 
+	/* At this point we should be pointing right at the "in" */
+	assert(!memcmp(ptr, "in", 2));
+	ptr += 2;
+
 	while (*ptr && isspace((unsigned char) *ptr))
 	    ptr++;
 
 	/*
 	 * Make a list with the remaining words
 	 */
-	forLst = Lst_Init(FALSE);
+	accumFor.lst = Lst_Init(FALSE);
 	buf = Buf_Init(0);
 	sub = Var_Subst(NULL, ptr, VAR_GLOBAL, FALSE);
 
 #define ADDWORD() \
 	Buf_AddBytes(buf, ptr - wrd, (Byte *) wrd), \
 	Buf_AddByte(buf, (Byte) '\0'), \
-	Lst_AtFront(forLst, (ClientData) Buf_GetAll(buf, &varlen)), \
+	Lst_AtFront(accumFor.lst, (ClientData) Buf_GetAll(buf, &varlen)), \
 	Buf_Destroy(buf, FALSE)
 
 	for (ptr = sub; *ptr && isspace((unsigned char) *ptr); ptr++)
@@ -198,16 +238,21 @@
 		while (*ptr && isspace((unsigned char) *ptr))
 		    ptr++;
 		wrd = ptr--;
+	    }
+	if (DEBUG(FOR)) {
+	    int i;
+	    for (i=0; i<accumFor.nvars; i++) {
+		(void) fprintf(stderr, "For: variable %s\n", accumFor.vars[i]);
 	    }
-	if (DEBUG(FOR))
-	    (void) fprintf(stderr, "For: Iterator %s List %s\n", forVar, sub);
+	    (void) fprintf(stderr, "For: list %s\n", sub);
+	}
 	if (ptr - wrd > 0)
 	    ADDWORD();
 	else
 	    Buf_Destroy(buf, TRUE);
 	free((Address) sub);
 
-	forBuf = Buf_Init(0);
+	accumFor.buf = Buf_Init(0);
 	forLevel++;
 	return 1;
     }
@@ -234,8 +279,8 @@
     }
 
     if (forLevel != 0) {
-	Buf_AddBytes(forBuf, strlen(line), (Byte *) line);
-	Buf_AddByte(forBuf, (Byte) '\n');
+	Buf_AddBytes(accumFor.buf, strlen(line), (Byte *) line);
+	Buf_AddByte(accumFor.buf, (Byte) '\n');
 	return 1;
     }
     else {
@@ -243,42 +288,11 @@
     }
 }
 
-/*-
- *-----------------------------------------------------------------------
- * ForExec --
- *	Expand the for loop for this index and push it in the Makefile
- *
- * Results:
- *	None.
- *
- * Side Effects:
- *	None.
- *
- *-----------------------------------------------------------------------
- */
-static int
-ForExec(namep, argp)
-    ClientData namep;
-    ClientData argp;
-{
-    char *name = (char *) namep;
-    For *arg = (For *) argp;
-    int len;
-    Var_Set(arg->var, name, VAR_GLOBAL);
-    if (DEBUG(FOR))
-	(void) fprintf(stderr, "--- %s = %s\n", arg->var, name);
-    Parse_FromString(Var_Subst(arg->var, (char *) Buf_GetAll(arg->buf, &len),
-			       VAR_GLOBAL, FALSE));
-    Var_Delete(arg->var, VAR_GLOBAL);
-
-    return 0;
-}
-
 
 /*-
  *-----------------------------------------------------------------------
  * For_Run --
- *	Run the for loop, immitating the actions of an include file
+ *	Run the for loop, imitating the actions of an include file
  *
  * Results:
  *	None.
@@ -292,19 +306,83 @@
 For_Run()
 {
     For arg;
+    LstNode ln;
+    char **values;
+    int i, done=0, len;
+    char *guy, *orig_guy, *old_guy;
+
+    if (accumFor.buf == NULL || accumFor.vars == NULL || accumFor.lst == NULL)
+	return;
+    arg = accumFor;
+    accumFor.buf = NULL;
+    accumFor.vars = NULL;
+    accumFor.nvars = 0;
+    accumFor.lst = NULL;
 
-    if (forVar == NULL || forBuf == NULL || forLst == NULL)
+    if (Lst_Open(arg.lst)!=SUCCESS)
 	return;
-    arg.var = forVar;
-    arg.buf = forBuf;
-    arg.lst = forLst;
-    forVar = NULL;
-    forBuf = NULL;
-    forLst = NULL;
 
-    Lst_ForEach(arg.lst, ForExec, (ClientData) &arg);
+    values = emalloc(arg.nvars * sizeof(char *));
+    
+    while (!done) {
+	/* 
+	 * due to the dumb way this is set up, this loop must run
+	 * backwards.
+	 */
+	for (i=arg.nvars-1; i>=0; i--) {
+	    ln = Lst_Next(arg.lst);
+	    if (ln==NILLNODE) {
+		if (i!=arg.nvars-1) {
+		    Parse_Error(PARSE_FATAL, 
+				"Not enough words in for substitution list");
+		}
+		done = 1;
+		break;
+	    } else {
+		values[i] = (char *) Lst_Datum(ln);
+	    }
+	}
+	if (done)
+	    break;
+
+	for (i=0; i<arg.nvars; i++) {
+	    Var_Set(arg.vars[i], values[i], VAR_GLOBAL);
+	    if (DEBUG(FOR))
+		(void) fprintf(stderr, "--- %s = %s\n", arg.vars[i], 
+			       values[i]);
+	}
+
+	/*
+	 * Hack, hack, kludge.
+	 * This is really ugly, but to do it any better way would require
+	 * making major changes to var.c, which I don't want to get into
+	 * yet. There is no mechanism for expanding some variables, only
+	 * for expanding a single variable. That should be corrected, but
+	 * not right away. (XXX)
+	 */
+	
+	guy = (char *) Buf_GetAll(arg.buf, &len);
+	orig_guy = guy;
+	for (i=0; i<arg.nvars; i++) {
+	    old_guy = guy;
+	    guy = Var_Subst(arg.vars[i], guy, VAR_GLOBAL, FALSE);
+	    if (old_guy != orig_guy) free(old_guy);
+	}
+	Parse_FromString(guy);
+
+	for (i=0; i<arg.nvars; i++)
+	    Var_Delete(arg.vars[i], VAR_GLOBAL);
+    }
+
+    free(values);
+
+    Lst_Close(arg.lst);
+
+    for (i=0; i<arg.nvars; i++) {
+	free(arg.vars[i]);
+    }
+    free(arg.vars);
 
-    free((Address)arg.var);
     Lst_Destroy(arg.lst, (void (*) __P((ClientData))) free);
     Buf_Destroy(arg.buf, TRUE);
 }
Index: usr.bin/make/make.1
===================================================================
RCS file: /cvsroot/basesrc/usr.bin/make/make.1,v
retrieving revision 1.38
diff -u -3 -r1.38 make.1
--- make.1	2000/02/08 12:56:28	1.38
+++ make.1	2000/04/16 13:49:21
@@ -843,7 +843,8 @@
 .Bl -tag -width Ds
 .It Xo
 .Ic \&.for
-.Ar variable
+.Ar variable 
+.Op Ar variable ...
 .Ic in
 .Ar expression
 .Xc
@@ -856,12 +857,17 @@
 .El
 After the for
 .Ic expression
-is evaluated, it is split into words. The
-iteration
-.Ic variable
-is successively set to each word, and substituted in the
+is evaluated, it is split into words. On each iteration of the loop, 
+one word is taken and assigned to each
+.Ic variable ,
+in order, and these
+.Ic variables
+are substituted into the
 .Ic make-rules
 inside the body of the for loop.
+The number of words must come out even; that is, if there are three
+iteration variables, the number of words provided must be a multiple
+of three.
 .Sh COMMENTS
 Comments begin with a hash
 .Pq Ql \&#
>Release-Note:
>Audit-Trail:
>Unformatted: