Subject: Re: bin/24420
To: None <tech-toolchain@netbsd.org>
From: Simon Gerraty <sjg@juniper.net>
List: tech-toolchain
Date: 04/07/2004 12:03:31
The patch below is not quite the same as that provided in the PR.
This version will generate a warning about extra .else's even in
blocks which are being skipped.  Added a unit test...

Index: cond.c

finalElse is sized MAXIF+1 so that we can set finalElse[condTop]
without first checking if condTop < MAXIF, this avoids the need to
repeat a bunch of tests.

===================================================================
RCS file: /cvsroot/src/usr.bin/make/cond.c,v
retrieving revision 1.19
diff -u -p -r1.19 cond.c
--- cond.c	6 Jan 2004 01:18:52 -0000	1.19
+++ cond.c	7 Apr 2004 18:59:19 -0000
@@ -173,6 +173,7 @@ static Token	  condPushBack=None;	/* Sin
 
 #define	MAXIF		30	  /* greatest depth of #if'ing */
 
+static Boolean	  finalElse[MAXIF+1]; 	/* Seen final else (stack) */
 static Boolean	  condStack[MAXIF]; 	/* Stack of conditionals's values */
 static int  	  condTop = MAXIF;  	/* Top-most conditional */
 static int  	  skipIfLevel=0;    	/* Depth of skipped conditionals */
@@ -1232,6 +1233,7 @@ Cond_Eval(char *line)
 	 * so we return COND_PARSE, unless this endif isn't paired with
 	 * a decent if.
 	 */
+	finalElse[condTop] = FALSE;
 	if (skipIfLevel != 0) {
 	    skipIfLevel -= 1;
 	    return (COND_SKIP);
@@ -1266,6 +1268,11 @@ Cond_Eval(char *line)
 	 * of the previous if we parsed.
 	 */
 	if (isElse && (line[0] == 's') && (line[1] == 'e')) {
+	    if (finalElse[condTop]) {
+		Parse_Error (PARSE_WARNING, "extra else");
+	    } else {
+		finalElse[condTop] = TRUE;
+	    }
 	    if (condTop == MAXIF) {
 		Parse_Error (level, "if-less else");
 		return (COND_INVALID);
@@ -1315,6 +1322,7 @@ Cond_Eval(char *line)
     }
     if (!isElse) {
 	condTop -= 1;
+	finalElse[condTop] = FALSE;
     } else if ((skipIfLevel != 0) || condStack[condTop]) {
 	/*
 	 * If this is an else-type conditional, it should only take effect
Index: unit-tests/Makefile
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/Makefile,v
retrieving revision 1.11
diff -u -p -r1.11 Makefile
--- unit-tests/Makefile	20 Feb 2004 09:03:26 -0000	1.11
+++ unit-tests/Makefile	7 Apr 2004 18:59:19 -0000
@@ -19,6 +19,7 @@ UNIT_TESTS:= ${.PARSEDIR}
 # Simple sub-makefiles - we run them as a black box
 # keep the list sorted.
 SUBFILES= \
+	cond1 \
 	modmatch \
 	modts \
 	modword \
@@ -40,12 +41,13 @@ clean:
 TEST_MAKE?= ${MAKE}
 
 # The driver.
-# We always pretend .MAKE was called 'make' so the results
-# can be compared.
+# We always pretend .MAKE was called 'make' 
+# and strip ${.CURDIR}/ from the output
+# so the results can be compared.
 test:
 	@echo "${TEST_MAKE} -f ${MAKEFILE} > ${.TARGET}.out 2>&1"
 	@cd ${.OBJDIR} && ${TEST_MAKE} -f ${MAKEFILE} 2>&1 | \
-	sed 's,^${TEST_MAKE:T}:,make:,' > ${.TARGET}.out || { \
+	sed -e 's,^${TEST_MAKE:T}:,make:,' -e 's,${.CURDIR}/,,g' > ${.TARGET}.out || { \
 	tail ${.TARGET}.out; mv ${.TARGET}.out ${.TARGET}.fail; exit 1; }
 	diff -u ${UNIT_TESTS}/${.TARGET}.exp ${.TARGET}.out
 
Index: unit-tests/cond1
===================================================================
RCS file: unit-tests/cond1
diff -N unit-tests/cond1
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ unit-tests/cond1	7 Apr 2004 18:59:19 -0000
@@ -0,0 +1,37 @@
+# $Id$
+
+PRIMES=2 3 5 7 11
+NUMBERS=1 2 3 4 5 6 7 8 9 10 11 12
+
+n=2
+.if ${PRIMES:M$n} == ""
+X=not
+.else
+X=
+.endif
+
+# We expect an extra else warning
+.if ${MACHINE_ARCH} == no-such
+A=one
+.else
+A=other
+.else
+A=this should be an error
+.endif
+
+# We expect an extra else warning
+.if $X != ""
+.if ${MACHINE_ARCH} == no-such
+B=one
+.else
+B=other
+.else
+B=this should be an error
+.endif
+.else
+B=unknown
+.endif
+
+all:
+	@echo "$n is $X prime"
+	@echo "A='$A' B='$B'"
Index: unit-tests/test.exp
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/test.exp,v
retrieving revision 1.8
diff -u -p -r1.8 test.exp
--- unit-tests/test.exp	20 Feb 2004 09:03:26 -0000	1.8
+++ unit-tests/test.exp	7 Apr 2004 18:59:19 -0000
@@ -1,3 +1,7 @@
+make: "cond1" line 18: warning: extra else
+make: "cond1" line 28: warning: extra else
+2 is  prime
+A='other' B='unknown'
 LIB=a X_LIBS:M${LIB${LIB:tu}} is "/tmp/liba.a"
 LIB=a X_LIBS:M*/lib${LIB}.a is "/tmp/liba.a"
 LIB=a X_LIBS:M*/lib${LIB}.a:tu is "/TMP/LIBA.A"