Date: Mon, 12 Jan 2004 10:24:44 -0500 (EST) From: Mikhail Teterin <mi@aldan.algebra.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/61257: improvement to make(1)'s diagnostic of mismatched .if-.endif Message-ID: <200401121524.i0CFOiXn047237@aldan.algebra.com> Resent-Message-ID: <200401121530.i0CFUCpn096654@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200401121524.i0CFOiXn047237>