Module Name:    src
Committed By:   rillig
Date:           Fri Aug 11 04:56:31 UTC 2023

Modified Files:
        src/usr.bin/make: cond.c
        src/usr.bin/make/unit-tests: directive-include-guard.exp
            directive-include-guard.mk

Log Message:
make: clean up multiple-inclusion guards

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.353 -r1.354 src/usr.bin/make/cond.c
cvs rdiff -u -r1.10 -r1.11 \
    src/usr.bin/make/unit-tests/directive-include-guard.exp
cvs rdiff -u -r1.11 -r1.12 \
    src/usr.bin/make/unit-tests/directive-include-guard.mk

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/usr.bin/make/cond.c
diff -u src/usr.bin/make/cond.c:1.353 src/usr.bin/make/cond.c:1.354
--- src/usr.bin/make/cond.c:1.353	Fri Jun 23 05:21:10 2023
+++ src/usr.bin/make/cond.c	Fri Aug 11 04:56:31 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: cond.c,v 1.353 2023/06/23 05:21:10 rillig Exp $	*/
+/*	$NetBSD: cond.c,v 1.354 2023/08/11 04:56:31 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990 The Regents of the University of California.
@@ -92,7 +92,7 @@
 #include "dir.h"
 
 /*	"@(#)cond.c	8.2 (Berkeley) 1/2/94"	*/
-MAKE_RCSID("$NetBSD: cond.c,v 1.353 2023/06/23 05:21:10 rillig Exp $");
+MAKE_RCSID("$NetBSD: cond.c,v 1.354 2023/08/11 04:56:31 rillig Exp $");
 
 /*
  * Conditional expressions conform to this grammar:
@@ -224,17 +224,13 @@ ParseWord(const char **pp, bool doEval)
 		if ((ch == '&' || ch == '|') && paren_depth == 0)
 			break;
 		if (ch == '$') {
-			/*
-			 * Parse the variable expression and install it as
-			 * part of the argument if it's valid. We tell
-			 * Var_Parse to complain on an undefined variable,
-			 * (XXX: but Var_Parse ignores that request)
-			 * so we don't need to do it. Nor do we return an
-			 * error, though perhaps we should.
-			 */
 			VarEvalMode emode = doEval
 			    ? VARE_UNDEFERR
 			    : VARE_PARSE_ONLY;
+			/*
+			 * TODO: make Var_Parse complain about undefined
+			 * variables.
+			 */
 			FStr nestedVal = Var_Parse(&p, SCOPE_CMDLINE, emode);
 			/* TODO: handle errors */
 			Buf_AddStr(&word, nestedVal.str);
@@ -1267,7 +1263,6 @@ Cond_ExtractGuard(const char *line)
 {
 	const char *p, *varname;
 	Substring dir;
-	enum GuardKind kind;
 	Guard *guard;
 
 	p = line + 1;		/* skip the '.' */
@@ -1288,10 +1283,9 @@ Cond_ExtractGuard(const char *line)
 			const char *arg_p = p;
 			free(ParseWord(&p, false));
 			if (strcmp(p, ")") == 0) {
-				char *target = ParseWord(&arg_p, true);
 				guard = bmake_malloc(sizeof(*guard));
 				guard->kind = GK_TARGET;
-				guard->name = target;
+				guard->name = ParseWord(&arg_p, true);
 				return guard;
 			}
 		}
@@ -1302,9 +1296,8 @@ Cond_ExtractGuard(const char *line)
 	return NULL;
 
 found_variable:
-	kind = GK_VARIABLE;
 	guard = bmake_malloc(sizeof(*guard));
-	guard->kind = kind;
+	guard->kind = GK_VARIABLE;
 	guard->name = bmake_strsedup(varname, p);
 	return guard;
 }

Index: src/usr.bin/make/unit-tests/directive-include-guard.exp
diff -u src/usr.bin/make/unit-tests/directive-include-guard.exp:1.10 src/usr.bin/make/unit-tests/directive-include-guard.exp:1.11
--- src/usr.bin/make/unit-tests/directive-include-guard.exp:1.10	Wed Jun 21 21:21:52 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.exp	Fri Aug 11 04:56:31 2023
@@ -51,14 +51,14 @@ Parse_PushInput: file variable-undef-ins
 Parse_PushInput: file variable-undef-inside.tmp, line 1
 Parse_PushInput: file variable-not-defined.tmp, line 1
 Parse_PushInput: file variable-not-defined.tmp, line 1
-Parse_PushInput: file if-elif.tmp, line 1
-Parse_PushInput: file if-elif.tmp, line 1
-Parse_PushInput: file if-elif-reuse.tmp, line 1
-Parse_PushInput: file if-elif-reuse.tmp, line 1
-Parse_PushInput: file if-else.tmp, line 1
-Parse_PushInput: file if-else.tmp, line 1
-Parse_PushInput: file if-else-reuse.tmp, line 1
-Parse_PushInput: file if-else-reuse.tmp, line 1
+Parse_PushInput: file elif.tmp, line 1
+Parse_PushInput: file elif.tmp, line 1
+Parse_PushInput: file elif-reuse.tmp, line 1
+Parse_PushInput: file elif-reuse.tmp, line 1
+Parse_PushInput: file else.tmp, line 1
+Parse_PushInput: file else.tmp, line 1
+Parse_PushInput: file else-reuse.tmp, line 1
+Parse_PushInput: file else-reuse.tmp, line 1
 Parse_PushInput: file inner-if-elif-else.tmp, line 1
 Skipping 'inner-if-elif-else.tmp' because 'INNER_IF_ELIF_ELSE' is defined
 Parse_PushInput: file target.tmp, line 1

Index: src/usr.bin/make/unit-tests/directive-include-guard.mk
diff -u src/usr.bin/make/unit-tests/directive-include-guard.mk:1.11 src/usr.bin/make/unit-tests/directive-include-guard.mk:1.12
--- src/usr.bin/make/unit-tests/directive-include-guard.mk:1.11	Wed Jun 21 21:21:52 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.mk	Fri Aug 11 04:56:31 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-include-guard.mk,v 1.11 2023/06/21 21:21:52 sjg Exp $
+# $NetBSD: directive-include-guard.mk,v 1.12 2023/08/11 04:56:31 rillig Exp $
 #
 # Tests for multiple-inclusion guards in makefiles.
 #
@@ -112,9 +112,10 @@ LINES.variable-name-mismatch= \
 
 # The variable name '!VARNAME' cannot be used in an '.ifndef' directive, as
 # the '!' would be a negation.  It is syntactically valid in a '.if !defined'
-# condition, but ignored there.  Furthermore, when defining the variable, the
-# character '!' has to be escaped, to prevent it from being interpreted as the
-# '!' dependency operator.
+# condition, but this case is so uncommon that the guard mechanism doesn't
+# accept '!' in the guard variable name. Furthermore, when defining the
+# variable, the character '!' has to be escaped, to prevent it from being
+# interpreted as the '!' dependency operator.
 INCS+=	variable-name-exclamation
 LINES.variable-name-exclamation= \
 	'.if !defined(!VARIABLE_NAME_EXCLAMATION)' \
@@ -189,9 +190,9 @@ LINES.variable-if-indirect= \
 # expect: Parse_PushInput: file variable-if-indirect.tmp, line 1
 
 # The variable name in the guard condition must only contain alphanumeric
-# characters and underscores.  The guard variable is more flexible, it can be
-# defined anywhere, as long as it is defined at the point where the file is
-# included the next time.
+# characters and underscores.  The place where the guard variable is defined
+# is more flexible, as long as the variable is defined at the point where the
+# file is included the next time.
 INCS+=	variable-assign-indirect
 LINES.variable-assign-indirect= \
 	'.ifndef VARIABLE_ASSIGN_INDIRECT' \
@@ -254,9 +255,9 @@ UNDEF_BETWEEN.variable-defined-then-unde
 # expect: Parse_PushInput: file variable-defined-then-undefined.tmp, line 1
 
 # The whole file content must be guarded by a single '.if' conditional, not by
-# several, even if they have the same effect.  This case is not expected to
-# occur in practice, as the two parts would rather be split into separate
-# files.
+# several, as each of these conditionals would require its separate guard.
+# This case is not expected to occur in practice, as the two parts would
+# rather be split into separate files.
 INCS+=	variable-two-times
 LINES.variable-two-times= \
 	'.ifndef VARIABLE_TWO_TIMES_1' \
@@ -324,46 +325,46 @@ LINES.variable-not-defined= \
 # expect: Parse_PushInput: file variable-not-defined.tmp, line 1
 
 # The outermost '.if' must not have an '.elif' branch.
-INCS+=	if-elif
-LINES.if-elif= \
-	'.ifndef IF_ELIF' \
-	'IF_ELIF=' \
+INCS+=	elif
+LINES.elif= \
+	'.ifndef ELIF' \
+	'ELIF=' \
 	'.elif 1' \
 	'.endif'
-# expect: Parse_PushInput: file if-elif.tmp, line 1
-# expect: Parse_PushInput: file if-elif.tmp, line 1
+# expect: Parse_PushInput: file elif.tmp, line 1
+# expect: Parse_PushInput: file elif.tmp, line 1
 
 # When a file with an '.if/.elif/.endif' conditional at the top level is
 # included, it is never optimized, as one of its branches is taken.
-INCS+=	if-elif-reuse
-LINES.if-elif-reuse= \
-	'.ifndef IF_ELIF' \
+INCS+=	elif-reuse
+LINES.elif-reuse= \
+	'.ifndef ELIF' \
 	'syntax error' \
 	'.elif 1' \
 	'.endif'
-# expect: Parse_PushInput: file if-elif-reuse.tmp, line 1
-# expect: Parse_PushInput: file if-elif-reuse.tmp, line 1
+# expect: Parse_PushInput: file elif-reuse.tmp, line 1
+# expect: Parse_PushInput: file elif-reuse.tmp, line 1
 
 # The outermost '.if' must not have an '.else' branch.
-INCS+=	if-else
-LINES.if-else= \
-	'.ifndef IF_ELSE' \
-	'IF_ELSE=' \
+INCS+=	else
+LINES.else= \
+	'.ifndef ELSE' \
+	'ELSE=' \
 	'.else' \
 	'.endif'
-# expect: Parse_PushInput: file if-else.tmp, line 1
-# expect: Parse_PushInput: file if-else.tmp, line 1
+# expect: Parse_PushInput: file else.tmp, line 1
+# expect: Parse_PushInput: file else.tmp, line 1
 
 # When a file with an '.if/.else/.endif' conditional at the top level is
 # included, it is never optimized, as one of its branches is taken.
-INCS+=	if-else-reuse
-LINES.if-else-reuse= \
-	'.ifndef IF_ELSE' \
+INCS+=	else-reuse
+LINES.else-reuse= \
+	'.ifndef ELSE' \
 	'syntax error' \
 	'.else' \
 	'.endif'
-# expect: Parse_PushInput: file if-else-reuse.tmp, line 1
-# expect: Parse_PushInput: file if-else-reuse.tmp, line 1
+# expect: Parse_PushInput: file else-reuse.tmp, line 1
+# expect: Parse_PushInput: file else-reuse.tmp, line 1
 
 # The inner '.if' directives may have an '.elif' or '.else', and it doesn't
 # matter which of their branches are taken.
@@ -451,17 +452,17 @@ LINES.target-indirect-PARSEFILE2= \
 
 # Using plain .PARSEFILE without .PARSEDIR leads to name clashes.  The include
 # guard is the same as in the test case 'target-indirect-PARSEFILE', as the
-# guard name only contains the basename but not the directory name.
+# guard name only contains the basename but not the directory name.  So even
+# without defining the guard variable, the file is considered guarded.
 INCS+=	subdir/target-indirect-PARSEFILE
 LINES.subdir/target-indirect-PARSEFILE= \
 	'.if !target(__$${.PARSEFILE}__)' \
-	'__$${.PARSEFILE}__: .NOTMAIN' \
 	'.endif'
 # expect: Parse_PushInput: file subdir/target-indirect-PARSEFILE.tmp, line 1
 # expect: Skipping 'subdir/target-indirect-PARSEFILE.tmp' because '__target-indirect-PARSEFILE.tmp__' is defined
 
 # Another common form of guard target is __${.PARSEDIR}/${.PARSEFILE}__
-# or __${.PARSEDIR:tA}/${.PARSEFILE}__ to be truely unique.
+# or __${.PARSEDIR:tA}/${.PARSEFILE}__ to be truly unique.
 INCS+=	target-indirect-PARSEDIR-PARSEFILE
 LINES.target-indirect-PARSEDIR-PARSEFILE= \
 	'.if !target(__$${.PARSEDIR}/$${.PARSEFILE}__)' \
@@ -503,7 +504,7 @@ LINES.target-plus= \
 # expect: Parse_PushInput: file target-plus.tmp, line 1
 
 # If the guard target is defined before the file is included the first time,
-# the file is not considered guarded.
+# the file is read once and then considered guarded.
 INCS+=	target-already-defined
 LINES.target-already-defined= \
 	'.if !target(target-already-defined)' \
@@ -530,6 +531,7 @@ LINES.target-name-exclamation= \
 # expect: Parse_PushInput: file target-name-exclamation.tmp, line 1
 # expect: Parse_PushInput: file target-name-exclamation.tmp, line 1
 
+
 # Now run all test cases by including each of the files twice and looking at
 # the debug output.  The files that properly guard against multiple inclusion
 # generate a 'Skipping' line, the others repeat the 'Parse_PushInput' line.

Reply via email to