Subject: bin/24420
To: None <lukem@netbsd.org>
From: Simon J. Gerraty <sjg@crufty.net>
List: tech-toolchain
Date: 04/07/2004 22:31:29
So, there were a couple of flaws with the original patch - or at least
my version of it.  If we're going to issue these warnings we should do
so for .else's that are being skipped as well - that was the point of
my variation from the original.   This means however that we need to
use a 2 dimensional array since condTop only gets bumped if we are not
already skipping.

I've added the .if's from bsd.obj.mk to the unit test to check for
false positive, that .if block is repeated to ensure the answer
doesn't change ;-) 

I've more testing to do but, looks ok so far.

--sjg

Index: cond.c
===================================================================
RCS file: /cvsroot/src/usr.bin/make/cond.c,v
retrieving revision 1.21
diff -u -p -r1.21 cond.c
--- cond.c	8 Apr 2004 01:35:33 -0000	1.21
+++ cond.c	8 Apr 2004 05:27:21 -0000
@@ -173,6 +173,7 @@ static Token	  condPushBack=None;	/* Sin
 
 #define	MAXIF		30	  /* greatest depth of #if'ing */
 
+static Boolean	  finalElse[MAXIF+1][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][skipIfLevel] = 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][skipIfLevel]) {
+		Parse_Error (PARSE_WARNING, "extra else");
+	    } else {
+		finalElse[condTop][skipIfLevel] = TRUE;
+	    }
 	    if (condTop == MAXIF) {
 		Parse_Error (level, "if-less else");
 		return (COND_INVALID);
@@ -1300,6 +1307,11 @@ Cond_Eval(char *line)
 	     * we're skipping things...
 	     */
 	    skipIfLevel += 1;
+	    if (skipIfLevel >= MAXIF) {
+		Parse_Error (PARSE_FATAL, "Too many nested if's. %d max.", MAXIF);
+		return (COND_INVALID);
+	    }
+	    finalElse[condTop][skipIfLevel] = FALSE;
 	    return(COND_SKIP);
 	}
 
@@ -1315,6 +1327,7 @@ Cond_Eval(char *line)
     }
     if (!isElse) {
 	condTop -= 1;
+	finalElse[condTop][skipIfLevel] = FALSE;
     } else if ((skipIfLevel != 0) || condStack[condTop]) {
 	/*
 	 * If this is an else-type conditional, it should only take effect
Index: unit-tests/cond1
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/cond1,v
retrieving revision 1.1
diff -u -p -r1.1 cond1
--- unit-tests/cond1	8 Apr 2004 00:59:01 -0000	1.1
+++ unit-tests/cond1	8 Apr 2004 05:27:21 -0000
@@ -1,5 +1,35 @@
 # $Id: cond1,v 1.1 2004/04/08 00:59:01 sjg Exp $
 
+
+# from bsd.obj.mk
+MKOBJ?=no
+.if ${MKOBJ} == "no"
+o= no
+.else
+.if defined(notMAKEOBJDIRPREFIX) || defined(norMAKEOBJDIR)
+.if defined(notMAKEOBJDIRPREFIX)
+o=${MAKEOBJDIRPREFIX}${__curdir}
+.else
+o= ${MAKEOBJDIR}
+.endif
+.endif
+o= o
+.endif
+
+# repeat the above to check we get the same result
+.if ${MKOBJ} == "no"
+o2= no
+.else
+.if defined(notMAKEOBJDIRPREFIX) || defined(norMAKEOBJDIR)
+.if defined(notMAKEOBJDIRPREFIX)
+o2=${MAKEOBJDIRPREFIX}${__curdir}
+.else
+o2= ${MAKEOBJDIR}
+.endif
+.endif
+o2= o
+.endif
+
 PRIMES=2 3 5 7 11
 NUMBERS=1 2 3 4 5
 
@@ -10,21 +40,28 @@ X=not
 X=
 .endif
 
-# We expect an extra else warning
 .if ${MACHINE_ARCH} == no-such
 A=one
 .else
+.if ${MACHINE_ARCH} == not-this
+.if ${MACHINE_ARCH} == something-else
+A=unlikely
+.else
+A=no
+.endif
+.endif
 A=other
+# We expect an extra else warning - we're not skipping here
 .else
 A=this should be an error
 .endif
 
-# We expect an extra else warning
 .if $X != ""
-.if ${MACHINE_ARCH} == no-such
+.if $X == not
 B=one
 .else
 B=other
+# We expect an extra else warning - we are skipping here
 .else
 B=this should be an error
 .endif
@@ -34,4 +71,4 @@ B=unknown
 
 all:
 	@echo "$n is $X prime"
-	@echo "A='$A' B='$B'"
+	@echo "A='$A' B='$B' o='$o,${o2}'"
Index: unit-tests/test.exp
===================================================================
RCS file: /cvsroot/src/usr.bin/make/unit-tests/test.exp,v
retrieving revision 1.9
diff -u -p -r1.9 test.exp
--- unit-tests/test.exp	8 Apr 2004 00:59:01 -0000	1.9
+++ unit-tests/test.exp	8 Apr 2004 05:27:21 -0000
@@ -1,7 +1,7 @@
-make: "cond1" line 18: warning: extra else
-make: "cond1" line 28: warning: extra else
+make: "cond1" line 55: warning: extra else
+make: "cond1" line 65: warning: extra else
 2 is  prime
-A='other' B='unknown'
+A='other' B='unknown' o='no,no'
 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"