Module Name: src
Committed By: rillig
Date: Wed Jun 21 14:33:36 UTC 2023
Modified Files:
src/usr.bin/make: parse.c
src/usr.bin/make/unit-tests: directive-include-guard.exp
directive-include-guard.mk
Log Message:
make: skip a file protected by a multiple-inclusion guard more often
In practice, the common situation is that a file is first included,
defines its multiple-inclusion guard and is then skipped instead of
being included again.
The other way round is that the multiple-inclusion guard is defined when
the file is included first. In that case, the file is now regarded as
guarded as well.
To generate a diff of this commit:
cvs rdiff -u -r1.702 -r1.703 src/usr.bin/make/parse.c
cvs rdiff -u -r1.8 -r1.9 \
src/usr.bin/make/unit-tests/directive-include-guard.exp
cvs rdiff -u -r1.9 -r1.10 \
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/parse.c
diff -u src/usr.bin/make/parse.c:1.702 src/usr.bin/make/parse.c:1.703
--- src/usr.bin/make/parse.c:1.702 Tue Jun 20 09:25:33 2023
+++ src/usr.bin/make/parse.c Wed Jun 21 14:33:36 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: parse.c,v 1.702 2023/06/20 09:25:33 rillig Exp $ */
+/* $NetBSD: parse.c,v 1.703 2023/06/21 14:33:36 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -105,7 +105,7 @@
#include "pathnames.h"
/* "@(#)parse.c 8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.702 2023/06/20 09:25:33 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.703 2023/06/21 14:33:36 rillig Exp $");
/* Detects a multiple-inclusion guard in a makefile. */
typedef enum {
@@ -2681,7 +2681,7 @@ ReadHighLevelLine(void)
condResult = Cond_EvalLine(line);
if (curFile->guardState == GS_START) {
Guard *guard;
- if (condResult == CR_TRUE
+ if (condResult != CR_ERROR
&& (guard = Cond_ExtractGuard(line)) != NULL) {
curFile->guardState = GS_COND;
curFile->guard = 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.8 src/usr.bin/make/unit-tests/directive-include-guard.exp:1.9
--- src/usr.bin/make/unit-tests/directive-include-guard.exp:1.8 Wed Jun 21 12:16:31 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.exp Wed Jun 21 14:33:36 2023
@@ -1,9 +1,13 @@
Parse_PushInput: file variable-ifndef.tmp, line 1
Skipping 'variable-ifndef.tmp' because 'VARIABLE_IFNDEF' is defined
+Parse_PushInput: file variable-ifndef-reuse.tmp, line 1
+Skipping 'variable-ifndef-reuse.tmp' because 'VARIABLE_IFNDEF' is defined
Parse_PushInput: file comments.tmp, line 1
Skipping 'comments.tmp' because 'COMMENTS' is defined
Parse_PushInput: file variable-if.tmp, line 1
Skipping 'variable-if.tmp' because 'VARIABLE_IF' is defined
+Parse_PushInput: file variable-if-reuse.tmp, line 1
+Skipping 'variable-if-reuse.tmp' because 'VARIABLE_IF' is defined
Parse_PushInput: file variable-if-triple-negation.tmp, line 1
Parse_PushInput: file variable-if-triple-negation.tmp, line 1
Parse_PushInput: file variable-ifdef-negated.tmp, line 1
@@ -32,11 +36,13 @@ Parse_PushInput: file variable-assign-ne
Parse_PushInput: .for loop in variable-assign-nested.tmp, line 3
Skipping 'variable-assign-nested.tmp' because 'VARIABLE_ASSIGN_NESTED' is defined
Parse_PushInput: file variable-already-defined.tmp, line 1
-Parse_PushInput: file variable-already-defined.tmp, line 1
+Skipping 'variable-already-defined.tmp' because 'VARIABLE_ALREADY_DEFINED' is defined
+Parse_PushInput: file variable-defined-then-undefined.tmp, line 1
+Parse_PushInput: file variable-defined-then-undefined.tmp, line 1
Parse_PushInput: file variable-two-times.tmp, line 1
Parse_PushInput: file variable-two-times.tmp, line 1
Parse_PushInput: file variable-clash.tmp, line 1
-Parse_PushInput: file variable-clash.tmp, line 1
+Skipping 'variable-clash.tmp' because 'VARIABLE_IF' is defined
Parse_PushInput: file variable-swapped.tmp, line 1
Parse_PushInput: file variable-swapped.tmp, line 1
Parse_PushInput: file variable-undef-between.tmp, line 1
@@ -47,8 +53,12 @@ Parse_PushInput: file variable-not-defin
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 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
@@ -62,13 +72,13 @@ Skipping 'target-indirect-PARSEFILE.tmp'
Parse_PushInput: file target-indirect-PARSEFILE2.tmp, line 1
Skipping 'target-indirect-PARSEFILE2.tmp' because '__target-indirect-PARSEFILE2.tmp__' is defined
Parse_PushInput: file subdir/target-indirect-PARSEFILE.tmp, line 1
-Parse_PushInput: file subdir/target-indirect-PARSEFILE.tmp, line 1
+Skipping 'subdir/target-indirect-PARSEFILE.tmp' because '__target-indirect-PARSEFILE.tmp__' is defined
Parse_PushInput: file target-indirect-PARSEFILE-tA.tmp, line 1
Skipping 'target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
Parse_PushInput: file subdir/target-indirect-PARSEFILE-tA.tmp, line 1
Skipping 'subdir/target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
Parse_PushInput: file subdir2/target-indirect-PARSEFILE-tA.tmp, line 1
-Parse_PushInput: file subdir2/target-indirect-PARSEFILE-tA.tmp, line 1
+Skipping 'subdir2/target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
Parse_PushInput: file target-indirect-PARSEDIR-PARSEFILE.tmp, line 1
Skipping 'target-indirect-PARSEDIR-PARSEFILE.tmp' because '__target-indirect-PARSEDIR-PARSEFILE.tmp__' is defined
Parse_PushInput: file subdir/target-indirect-PARSEDIR-PARSEFILE.tmp, line 1
@@ -77,8 +87,8 @@ Parse_PushInput: file target-unguarded.t
Parse_PushInput: file target-unguarded.tmp, line 1
Parse_PushInput: file target-plus.tmp, line 1
Parse_PushInput: file target-plus.tmp, line 1
-Parse_PushInput: file target-already-set.tmp, line 1
-Parse_PushInput: file target-already-set.tmp, line 1
+Parse_PushInput: file target-already-defined.tmp, line 1
+Skipping 'target-already-defined.tmp' because 'target-already-defined' is defined
Parse_PushInput: file target-name-exclamation.tmp, line 1
Parse_PushInput: file target-name-exclamation.tmp, line 1
exit status 0
Index: src/usr.bin/make/unit-tests/directive-include-guard.mk
diff -u src/usr.bin/make/unit-tests/directive-include-guard.mk:1.9 src/usr.bin/make/unit-tests/directive-include-guard.mk:1.10
--- src/usr.bin/make/unit-tests/directive-include-guard.mk:1.9 Wed Jun 21 12:16:31 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.mk Wed Jun 21 14:33:36 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-include-guard.mk,v 1.9 2023/06/21 12:16:31 rillig Exp $
+# $NetBSD: directive-include-guard.mk,v 1.10 2023/06/21 14:33:36 rillig Exp $
#
# Tests for multiple-inclusion guards in makefiles.
#
@@ -15,8 +15,8 @@
# .endif
#
# When such a file is included for the second or later time, and the guard
-# variable is set or the guard target defined, including the file has no
-# effect, as all its content is skipped.
+# variable or the guard target is defined, including the file has no effect,
+# as all its content is skipped.
#
# See also:
# https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
@@ -35,6 +35,17 @@ LINES.variable-ifndef= \
# expect: Parse_PushInput: file variable-ifndef.tmp, line 1
# expect: Skipping 'variable-ifndef.tmp' because 'VARIABLE_IFNDEF' is defined
+# A file that reuses a guard from a previous file (or whose guard is defined
+# for any other reason) is only processed once, to see whether it is guarded.
+# Its content is skipped, therefore the syntax error is not detected.
+INCS+= variable-ifndef-reuse
+LINES.variable-ifndef-reuse= \
+ '.ifndef VARIABLE_IFNDEF' \
+ 'syntax error' \
+ '.endif'
+# expect: Parse_PushInput: file variable-ifndef-reuse.tmp, line 1
+# expect: Skipping 'variable-ifndef-reuse.tmp' because 'VARIABLE_IFNDEF' is defined
+
# Comments and empty lines do not affect the multiple-inclusion guard.
INCS+= comments
LINES.comments= \
@@ -59,6 +70,17 @@ LINES.variable-if= \
# expect: Parse_PushInput: file variable-if.tmp, line 1
# expect: Skipping 'variable-if.tmp' because 'VARIABLE_IF' is defined
+# A file that reuses a guard from a previous file (or whose guard is defined
+# for any other reason) is only processed once, to see whether it is guarded.
+# Its content is skipped, therefore the syntax error is not detected.
+INCS+= variable-if-reuse
+LINES.variable-if-reuse= \
+ '.if !defined(VARIABLE_IF)' \
+ 'syntax error' \
+ '.endif'
+# expect: Parse_PushInput: file variable-if-reuse.tmp, line 1
+# expect: Skipping 'variable-if-reuse.tmp' because 'VARIABLE_IF' is defined
+
# Triple negation is so uncommon that it's not recognized, even though it has
# the same effect as a single negation.
INCS+= variable-if-triple-negation
@@ -168,7 +190,8 @@ LINES.variable-if-indirect= \
# The variable name in the guard condition must only contain alphanumeric
# characters and underscores. The guard variable is more flexible, it can be
-# set anywhere, as long as it is set when the guarded file is included next.
+# defined anywhere, as long as it 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' \
@@ -177,8 +200,8 @@ LINES.variable-assign-indirect= \
# expect: Parse_PushInput: file variable-assign-indirect.tmp, line 1
# expect: Skipping 'variable-assign-indirect.tmp' because 'VARIABLE_ASSIGN_INDIRECT' is defined
-# The time at which the guard variable is set doesn't matter, as long as it is
-# set when the file is included the next time.
+# The time at which the guard variable is defined doesn't matter, as long as
+# it is defined at the point where the file is included the next time.
INCS+= variable-assign-late
LINES.variable-assign-late= \
'.ifndef VARIABLE_ASSIGN_LATE' \
@@ -188,8 +211,8 @@ LINES.variable-assign-late= \
# expect: Parse_PushInput: file variable-assign-late.tmp, line 1
# expect: Skipping 'variable-assign-late.tmp' because 'VARIABLE_ASSIGN_LATE' is defined
-# The time at which the guard variable is set doesn't matter, as long as it is
-# set when the file is included the next time.
+# The time at which the guard variable is defined doesn't matter, as long as
+# it is defined at the point where the file is included the next time.
INCS+= variable-assign-nested
LINES.variable-assign-nested= \
'.ifndef VARIABLE_ASSIGN_NESTED' \
@@ -203,8 +226,10 @@ LINES.variable-assign-nested= \
# expect: Skipping 'variable-assign-nested.tmp' because 'VARIABLE_ASSIGN_NESTED' is defined
# If the guard variable is defined before the file is included for the first
-# time, the file is not considered guarded. This behavior is not finally
-# decided yet, as it is only a consequence of the current implementation.
+# time, the file is considered guarded as well. In such a case, the parser
+# skips almost all lines, as they are irrelevant, but the structure of the
+# top-level '.if/.endif' conditional can be determined reliably enough to
+# decide whether the file is guarded.
INCS+= variable-already-defined
LINES.variable-already-defined= \
'.ifndef VARIABLE_ALREADY_DEFINED' \
@@ -212,7 +237,21 @@ LINES.variable-already-defined= \
'.endif'
VARIABLE_ALREADY_DEFINED=
# expect: Parse_PushInput: file variable-already-defined.tmp, line 1
-# expect: Parse_PushInput: file variable-already-defined.tmp, line 1
+# expect: Skipping 'variable-already-defined.tmp' because 'VARIABLE_ALREADY_DEFINED' is defined
+
+# If the guard variable is defined before the file is included the first time,
+# the file is processed but its content is skipped. If that same guard
+# variable is undefined when the file is included the second time, the file is
+# processed as usual.
+INCS+= variable-defined-then-undefined
+LINES.variable-defined-then-undefined= \
+ '.ifndef VARIABLE_DEFINED_THEN_UNDEFINED' \
+ '.endif'
+VARIABLE_DEFINED_THEN_UNDEFINED=
+UNDEF_BETWEEN.variable-defined-then-undefined= \
+ VARIABLE_DEFINED_THEN_UNDEFINED
+# expect: Parse_PushInput: file variable-defined-then-undefined.tmp, line 1
+# 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
@@ -230,8 +269,8 @@ LINES.variable-two-times= \
# expect: Parse_PushInput: file variable-two-times.tmp, line 1
# When multiple files use the same guard variable name, the optimization of
-# skipping the file only affects the file that is included first. The content
-# of the other files is still read but skipped, these files are not optimized.
+# skipping the file affects each of these files.
+#
# Choosing unique guard names is the responsibility of the makefile authors.
# A typical pattern of guard variable names is '${PROJECT}_${DIR}_${FILE}_MK'.
# System-provided files typically start the guard names with '_'.
@@ -239,7 +278,7 @@ INCS+= variable-clash
LINES.variable-clash= \
${LINES.variable-if}
# expect: Parse_PushInput: file variable-clash.tmp, line 1
-# expect: Parse_PushInput: file variable-clash.tmp, line 1
+# expect: Skipping 'variable-clash.tmp' because 'VARIABLE_IF' is defined
# The conditional must come before the assignment, otherwise the conditional
# is useless, as it always evaluates to false.
@@ -286,7 +325,7 @@ LINES.variable-not-defined= \
# The outermost '.if' must not have an '.elif' branch.
INCS+= if-elif
-LINES.if-elif = \
+LINES.if-elif= \
'.ifndef IF_ELIF' \
'IF_ELIF=' \
'.elif 1' \
@@ -294,9 +333,20 @@ LINES.if-elif = \
# expect: Parse_PushInput: file if-elif.tmp, line 1
# expect: Parse_PushInput: file if-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' \
+ '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
+
# The outermost '.if' must not have an '.else' branch.
INCS+= if-else
-LINES.if-else = \
+LINES.if-else= \
'.ifndef IF_ELSE' \
'IF_ELSE=' \
'.else' \
@@ -304,10 +354,21 @@ LINES.if-else = \
# expect: Parse_PushInput: file if-else.tmp, line 1
# expect: Parse_PushInput: file if-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' \
+ 'syntax error' \
+ '.else' \
+ '.endif'
+# expect: Parse_PushInput: file if-else-reuse.tmp, line 1
+# expect: Parse_PushInput: file if-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.
INCS+= inner-if-elif-else
-LINES.inner-if-elif-else = \
+LINES.inner-if-elif-else= \
'.ifndef INNER_IF_ELIF_ELSE' \
'INNER_IF_ELIF_ELSE=' \
'. if 0' \
@@ -389,15 +450,15 @@ LINES.target-indirect-PARSEFILE2= \
# expect: Skipping 'target-indirect-PARSEFILE2.tmp' because '__target-indirect-PARSEFILE2.tmp__' is defined
# Using plain .PARSEFILE without .PARSEDIR leads to name clashes. The include
-# guard doesn't step in here because the test case 'target-indirect-PARSEFILE'
-# already took the same guard name.
+# 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.
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: 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 __${.PARSEFILE:tA}__. This form only
# works for files that are in the current working directory, it does not work
@@ -438,7 +499,7 @@ LINES.subdir2/target-indirect-PARSEFILE-
'__$${.PARSEFILE:tA}__: .NOTMAIN' \
'.endif'
# expect: Parse_PushInput: file subdir2/target-indirect-PARSEFILE-tA.tmp, line 1
-# expect: Parse_PushInput: file subdir2/target-indirect-PARSEFILE-tA.tmp, line 1
+# expect: Skipping 'subdir2/target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
# Using both '.PARSEDIR' and '.PARSEFILE' to form the guard target name is a
# robust approach.
@@ -484,14 +545,14 @@ LINES.target-plus= \
# If the guard target is defined before the file is included the first time,
# the file is not considered guarded.
-INCS+= target-already-set
-LINES.target-already-set= \
- '.if !target(target-already-set)' \
- 'target-already-set: .NOTMAIN' \
- '.endif'
-target-already-set: .NOTMAIN
-# expect: Parse_PushInput: file target-already-set.tmp, line 1
-# expect: Parse_PushInput: file target-already-set.tmp, line 1
+INCS+= target-already-defined
+LINES.target-already-defined= \
+ '.if !target(target-already-defined)' \
+ 'target-already-defined: .NOTMAIN' \
+ '.endif'
+target-already-defined: .NOTMAIN
+# expect: Parse_PushInput: file target-already-defined.tmp, line 1
+# expect: Skipping 'target-already-defined.tmp' because 'target-already-defined' is defined
# A target name cannot contain the character '!'. In the condition, the '!'
# is syntactically valid, but in the dependency declaration line, the '!' is
