bin/61257: improvement to make(1)'s diagnostic of mismatched .if-.endif

[Mikhail Teterin]

>Number:         61257
>Category:       bin
>Synopsis:       improvement to make(1)'s diagnostic of mismatched .if-.endif
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Mon Jan 12 07:30:11 PST 2004
>Closed-Date:
>Last-Modified:
>Originator:     Mikhail Teterin
>Release:        FreeBSD 5.2-CURRENT i386
>Organization:
Virtual Estates, Inc.
>Environment:
System: FreeBSD aldan.algebra.com 5.2-CURRENT FreeBSD 5.2-CURRENT #6: Tue Jan 6 09:55:21 EST 2004 root at aldan.algebra.com:/ccd/obj/ccd/src/sys/DEBUG i386

>Description:
	
	When make finds a mismatch in a Makefile's .if-.else-.endif
	tree, the reported error does not help resolve the problem
	much, because it only contains the last line.

	The included patch keeps the line numbers of the .if-s on
	the same stack, where the current version keeps just the
	results of the evaluations of conditionals. In case of a
	mismatch it makes a neat, indented printout of the stack.

	Interestingly, this does not use any more of the make's
	memory. The stack used to consist of booleans (which are,
	in fact, ints). It now consists of ints using only one
	bit (positive/negative) to represent the boolean (as the
	maker intended it to be), leaving the other 31 bits to
	hold the line number.

	The only potential problem I can see with the patch, is that
	it exposes the lineno, which was previously static to parse.c,
	to the rest of the program. I don't think, this is a big
	deal, however.

>How-To-Repeat:

	Consider the following buggy makefile:
	.if 1
	A=a
	.else
	B=b

	The current make will report only:
	"Makefile", line 4: 1 open conditional

	My version will say:
	"Makefile", line 4: 1 open conditional:
	"Makefile", line 4:   at line 1 (evaluated to true)

	The benefits are more apparent with hairier makefiles, such
	the bsd.* and those of some ports.

>Fix:

	The patch below tested with many buildworlds on several of
	my machines:

Index: cond.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/cond.c,v
retrieving revision 1.27
diff -U2 -r1.27 cond.c
--- cond.c	7 Sep 2003 02:16:10 -0000	1.27
+++ cond.c	12 Jan 2004 15:05:21 -0000
@@ -133,5 +133,5 @@
 #define	MAXIF		30	  /* greatest depth of #if'ing */
 
-static Boolean	  condStack[MAXIF]; 	/* Stack of conditionals's values */
+static int	  condStack[MAXIF]; 	/* Stack of conditionals's values */
 static int  	  condTop = MAXIF;  	/* Top-most conditional */
 static int  	  skipIfLevel=0;    	/* Depth of skipped conditionals */
@@ -1046,7 +1046,8 @@
     struct If	    *ifp;
     Boolean 	    isElse;
-    Boolean 	    value = FALSE;
+    int 	    value;
     int	    	    level;  	/* Level at which to report errors. */
 
+    value = -lineno;
     level = PARSE_FATAL;
 
@@ -1109,5 +1110,5 @@
 		return (COND_INVALID);
 	    } else if (skipIfLevel == 0) {
-		value = !condStack[condTop];
+		value = -condStack[condTop];
 	    } else {
 		return (COND_SKIP);
@@ -1160,5 +1161,5 @@
 	    case True:
 		if (CondToken(TRUE) == EndOfFile) {
-		    value = TRUE;
+		    value = lineno;
 		    break;
 		}
@@ -1167,5 +1168,5 @@
 	    case False:
 		if (CondToken(TRUE) == EndOfFile) {
-		    value = FALSE;
+		    value = -lineno;
 		    break;
 		}
@@ -1182,5 +1183,5 @@
     if (!isElse) {
 	condTop -= 1;
-    } else if ((skipIfLevel != 0) || condStack[condTop]) {
+    } else if ((skipIfLevel != 0) || condStack[condTop] > 0) {
 	/*
 	 * If this is an else-type conditional, it should only take effect
@@ -1203,6 +1204,6 @@
     } else {
 	condStack[condTop] = value;
-	skipLine = !value;
-	return (value ? COND_PARSE : COND_SKIP);
+	skipLine = value < 0;
+	return (value > 0 ? COND_PARSE : COND_SKIP);
     }
 }
@@ -1225,6 +1226,17 @@
 {
     if (condTop != MAXIF) {
-	Parse_Error(PARSE_FATAL, "%d open conditional%s", MAXIF-condTop,
+	int	level;
+	char	spaces[MAXIF];
+	Parse_Error(PARSE_FATAL, "%d open conditional%s:", MAXIF-condTop,
 		    MAXIF-condTop == 1 ? "" : "s");
+	memset(spaces, ' ', sizeof(spaces));
+	for (level = condTop; level < MAXIF; level++) {
+	    if (condStack[level] < 0)
+		Parse_Error(PARSE_FATAL, "\t%.*sat line %d "
+		    "(evaluated to false)", MAXIF - level, spaces, -condStack[level]);
+	    else
+		Parse_Error(PARSE_FATAL, "\t%.*sat line %d "
+		    "(evaluated to true)", MAXIF - level, spaces, condStack[level]);
+	}
     }
     condTop = MAXIF;
Index: make.h
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/make.h,v
retrieving revision 1.23
diff -U2 -r1.23 make.h
--- make.h	10 Oct 2002 19:27:48 -0000	1.23
+++ make.h	12 Jan 2004 15:05:24 -0000
@@ -274,4 +274,5 @@
 extern Lst     	dirSearchPath; 	/* The list of directories to search when
 				 * looking for targets */
+extern int	lineno;     	/* line number in current file */
 extern Lst	parseIncPath;	/* The list of directories to search when
 				 * looking for includes */
Index: parse.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/make/parse.c,v
retrieving revision 1.50
diff -U2 -r1.50 parse.c
--- parse.c	28 Nov 2002 12:47:56 -0000	1.50
+++ parse.c	12 Jan 2004 15:05:37 -0000
@@ -112,5 +112,5 @@
 
 static char    	    *fname;	/* name of current file (for errors) */
-static int          lineno;	/* line number in current file */
+int                 lineno;	/* line number in current file */
 static FILE   	    *curFILE = NULL; 	/* current makefile */
 
>Release-Note:
>Audit-Trail:
>Unformatted: