Module Name:    src
Committed By:   rillig
Date:           Wed Jun 21 12:16:31 UTC 2023

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

Log Message:
tests/make: clean up and extend tests for multiple-inclusion guards

Multiple-inclusion guards can be defined either as variables or as
targets.  Rename the variable tests so they include the word 'variable'.

Add tests to cover special characters in guard names (both variable and
target), just in case ParseVarnameGuard gets removed someday.

Document the pitfalls associated with choosing a naming scheme for
guards that leads to name clashes, such as with .PARSEFILE without
.PARSEDIR.


To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 \
    src/usr.bin/make/unit-tests/directive-include-guard.exp
cvs rdiff -u -r1.8 -r1.9 \
    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/unit-tests/directive-include-guard.exp
diff -u src/usr.bin/make/unit-tests/directive-include-guard.exp:1.7 src/usr.bin/make/unit-tests/directive-include-guard.exp:1.8
--- src/usr.bin/make/unit-tests/directive-include-guard.exp:1.7	Wed Jun 21 04:20:21 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.exp	Wed Jun 21 12:16:31 2023
@@ -1,41 +1,50 @@
-Parse_PushInput: file guarded-ifndef.tmp, line 1
-Skipping 'guarded-ifndef.tmp' because 'GUARDED_IFNDEF' is defined
+Parse_PushInput: file variable-ifndef.tmp, line 1
+Skipping 'variable-ifndef.tmp' because 'VARIABLE_IFNDEF' is defined
 Parse_PushInput: file comments.tmp, line 1
 Skipping 'comments.tmp' because 'COMMENTS' is defined
-Parse_PushInput: file guarded-if.tmp, line 1
-Skipping 'guarded-if.tmp' because 'GUARDED_IF' is defined
-Parse_PushInput: file triple-negation.tmp, line 1
-Parse_PushInput: file triple-negation.tmp, line 1
-Parse_PushInput: file ifdef-negated.tmp, line 1
-Parse_PushInput: file ifdef-negated.tmp, line 1
-Parse_PushInput: file varname-mismatch.tmp, line 1
-Parse_PushInput: file varname-mismatch.tmp, line 1
-Parse_PushInput: file ifndef-plus.tmp, line 1
-Parse_PushInput: file ifndef-plus.tmp, line 1
-Parse_PushInput: file if-plus.tmp, line 1
-Parse_PushInput: file if-plus.tmp, line 1
-Parse_PushInput: file ifndef-indirect.tmp, line 1
-Parse_PushInput: file ifndef-indirect.tmp, line 1
-Parse_PushInput: file if-indirect.tmp, line 1
-Parse_PushInput: file if-indirect.tmp, line 1
-Parse_PushInput: file varassign-indirect.tmp, line 1
-Skipping 'varassign-indirect.tmp' because 'VARASSIGN_INDIRECT' is defined
-Parse_PushInput: file late-assignment.tmp, line 1
-Skipping 'late-assignment.tmp' because 'LATE_ASSIGNMENT' is defined
-Parse_PushInput: file two-conditions.tmp, line 1
-Skipping 'two-conditions.tmp' because 'TWO_CONDITIONS' is defined
-Parse_PushInput: file already-set.tmp, line 1
-Parse_PushInput: file already-set.tmp, line 1
-Parse_PushInput: file twice.tmp, line 1
-Parse_PushInput: file twice.tmp, line 1
-Parse_PushInput: file reuse.tmp, line 1
-Parse_PushInput: file reuse.tmp, line 1
-Parse_PushInput: file swapped.tmp, line 1
-Parse_PushInput: file swapped.tmp, line 1
-Parse_PushInput: file undef-between.tmp, line 1
-Parse_PushInput: file undef-between.tmp, line 1
-Parse_PushInput: file undef-inside.tmp, line 1
-Parse_PushInput: file undef-inside.tmp, line 1
+Parse_PushInput: file variable-if.tmp, line 1
+Skipping 'variable-if.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
+Parse_PushInput: file variable-ifdef-negated.tmp, line 1
+Parse_PushInput: file variable-name-mismatch.tmp, line 1
+Parse_PushInput: file variable-name-mismatch.tmp, line 1
+Parse_PushInput: file variable-name-exclamation.tmp, line 1
+Parse_PushInput: file variable-name-exclamation.tmp, line 1
+Parse_PushInput: file variable-name-exclamation-middle.tmp, line 1
+Parse_PushInput: file variable-name-exclamation-middle.tmp, line 1
+Parse_PushInput: file variable-name-parentheses.tmp, line 1
+Parse_PushInput: file variable-name-parentheses.tmp, line 1
+Parse_PushInput: file variable-ifndef-plus.tmp, line 1
+Parse_PushInput: file variable-ifndef-plus.tmp, line 1
+Parse_PushInput: file variable-if-plus.tmp, line 1
+Parse_PushInput: file variable-if-plus.tmp, line 1
+Parse_PushInput: file variable-ifndef-indirect.tmp, line 1
+Parse_PushInput: file variable-ifndef-indirect.tmp, line 1
+Parse_PushInput: file variable-if-indirect.tmp, line 1
+Parse_PushInput: file variable-if-indirect.tmp, line 1
+Parse_PushInput: file variable-assign-indirect.tmp, line 1
+Skipping 'variable-assign-indirect.tmp' because 'VARIABLE_ASSIGN_INDIRECT' is defined
+Parse_PushInput: file variable-assign-late.tmp, line 1
+Skipping 'variable-assign-late.tmp' because 'VARIABLE_ASSIGN_LATE' is defined
+Parse_PushInput: file variable-assign-nested.tmp, line 1
+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
+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
+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
+Parse_PushInput: file variable-undef-between.tmp, line 1
+Parse_PushInput: file variable-undef-inside.tmp, line 1
+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-else.tmp, line 1
@@ -52,12 +61,24 @@ Parse_PushInput: file target-indirect-PA
 Skipping 'target-indirect-PARSEFILE.tmp' because '__target-indirect-PARSEFILE.tmp__' is defined
 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
 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
+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
+Skipping 'subdir/target-indirect-PARSEDIR-PARSEFILE.tmp' because '__subdir/target-indirect-PARSEDIR-PARSEFILE.tmp__' is defined
 Parse_PushInput: file target-unguarded.tmp, line 1
 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-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.8 src/usr.bin/make/unit-tests/directive-include-guard.mk:1.9
--- src/usr.bin/make/unit-tests/directive-include-guard.mk:1.8	Wed Jun 21 04:20:21 2023
+++ src/usr.bin/make/unit-tests/directive-include-guard.mk	Wed Jun 21 12:16:31 2023
@@ -1,4 +1,4 @@
-# $NetBSD: directive-include-guard.mk,v 1.8 2023/06/21 04:20:21 sjg Exp $
+# $NetBSD: directive-include-guard.mk,v 1.9 2023/06/21 12:16:31 rillig Exp $
 #
 # Tests for multiple-inclusion guards in makefiles.
 #
@@ -21,17 +21,21 @@
 # See also:
 #	https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
 
+# Each of the following test cases creates a temporary file named after the
+# test case and writes some lines of text to that file.  That file is then
+# included twice, to see whether the second '.include' is skipped.
 
-# This is the canonical form of a multiple-inclusion guard.
-INCS+=	guarded-ifndef
-LINES.guarded-ifndef= \
-	'.ifndef GUARDED_IFNDEF' \
-	'GUARDED_IFNDEF=' \
+
+# This is the canonical form of a variable-based multiple-inclusion guard.
+INCS+=	variable-ifndef
+LINES.variable-ifndef= \
+	'.ifndef VARIABLE_IFNDEF' \
+	'VARIABLE_IFNDEF=' \
 	'.endif'
-# expect: Parse_PushInput: file guarded-ifndef.tmp, line 1
-# expect: Skipping 'guarded-ifndef.tmp' because 'GUARDED_IFNDEF' is defined
+# expect: Parse_PushInput: file variable-ifndef.tmp, line 1
+# expect: Skipping 'variable-ifndef.tmp' because 'VARIABLE_IFNDEF' is defined
 
-# Comments and empty lines have no influence on the multiple-inclusion guard.
+# Comments and empty lines do not affect the multiple-inclusion guard.
 INCS+=	comments
 LINES.comments= \
 	'\# comment' \
@@ -45,183 +49,240 @@ LINES.comments= \
 # expect: Skipping 'comments.tmp' because 'COMMENTS' is defined
 
 # An alternative form uses the 'defined' function.  It is more verbose than
-# the canonical form.  There are other possible forms as well, such as with a
-# triple negation, but these are not recognized as they are not common.
-INCS+=	guarded-if
-LINES.guarded-if= \
-	'.if !defined(GUARDED_IF)' \
-	'GUARDED_IF=' \
-	'.endif'
-# expect: Parse_PushInput: file guarded-if.tmp, line 1
-# expect: Skipping 'guarded-if.tmp' because 'GUARDED_IF' is defined
-
-# Triple negation is so uncommon that it's not recognized.
-INCS+=	triple-negation
-LINES.triple-negation= \
-	'.if !!!defined(TRIPLE_NEGATION)' \
-	'TRIPLE_NEGATION=' \
-	'.endif'
-# expect: Parse_PushInput: file triple-negation.tmp, line 1
-# expect: Parse_PushInput: file triple-negation.tmp, line 1
-
-# A conditional other than '.if' or '.ifndef' marks the file as non-guarded,
-# even if it would actually work as a multiple-inclusion guard.
-INCS+=	ifdef-negated
-LINES.ifdef-negated= \
-	'.ifdef !IFDEF_NEGATED' \
-	'IFDEF_NEGATED=' \
+# the canonical form but avoids the '.ifndef' directive, as that directive is
+# not commonly used.
+INCS+=	variable-if
+LINES.variable-if= \
+	'.if !defined(VARIABLE_IF)' \
+	'VARIABLE_IF=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-if.tmp, line 1
+# expect: Skipping 'variable-if.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
+LINES.variable-if-triple-negation= \
+	'.if !!!defined(VARIABLE_IF_TRIPLE_NEGATION)' \
+	'VARIABLE_IF_TRIPLE_NEGATION=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-if-triple-negation.tmp, line 1
+# expect: Parse_PushInput: file variable-if-triple-negation.tmp, line 1
+
+# A conditional other than '.if' or '.ifndef' does not guard the file, even if
+# it is otherwise equivalent to the above accepted forms.
+INCS+=	variable-ifdef-negated
+LINES.variable-ifdef-negated= \
+	'.ifdef !VARIABLE_IFDEF_NEGATED' \
+	'VARIABLE_IFDEF_NEGATED=' \
 	'.endif'
-# expect: Parse_PushInput: file ifdef-negated.tmp, line 1
-# expect: Parse_PushInput: file ifdef-negated.tmp, line 1
+# expect: Parse_PushInput: file variable-ifdef-negated.tmp, line 1
+# expect: Parse_PushInput: file variable-ifdef-negated.tmp, line 1
 
 # The variable names in the '.if' and the assignment must be the same.
-INCS+=	varname-mismatch
-LINES.varname-mismatch= \
-	'.ifndef VARNAME_MISMATCH' \
-	'OTHER_NAME=' \
+INCS+=	variable-name-mismatch
+LINES.variable-name-mismatch= \
+	'.ifndef VARIABLE_NAME_MISMATCH' \
+	'VARIABLE_NAME_DIFFERENT=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-name-mismatch.tmp, line 1
+# expect: Parse_PushInput: file variable-name-mismatch.tmp, line 1
+
+# 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.
+INCS+=	variable-name-exclamation
+LINES.variable-name-exclamation= \
+	'.if !defined(!VARIABLE_NAME_EXCLAMATION)' \
+	'${:U!}VARIABLE_NAME_EXCLAMATION=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-name-exclamation.tmp, line 1
+# expect: Parse_PushInput: file variable-name-exclamation.tmp, line 1
+
+# A variable name can contain a '!' in the middle, as that character is
+# interpreted as an ordinary character in conditions as well as on the left
+# side of a variable assignment.  For guard variable names, the '!' is not
+# supported in any place, though.
+INCS+=	variable-name-exclamation-middle
+LINES.variable-name-exclamation-middle= \
+	'.ifndef VARIABLE_NAME!MIDDLE' \
+	'VARIABLE_NAME!MIDDLE=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-name-exclamation-middle.tmp, line 1
+# expect: Parse_PushInput: file variable-name-exclamation-middle.tmp, line 1
+
+# A variable name can contain balanced parentheses, at least in conditions and
+# on the left side of a variable assignment.  There are enough places in make
+# where parentheses or braces are handled inconsistently to make this naming
+# choice a bad idea, therefore these characters are not allowed in guard
+# variable names.
+INCS+=	variable-name-parentheses
+LINES.variable-name-parentheses= \
+	'.ifndef VARIABLE_NAME(&)PARENTHESES' \
+	'VARIABLE_NAME(&)PARENTHESES=' \
 	'.endif'
-# expect: Parse_PushInput: file varname-mismatch.tmp, line 1
-# expect: Parse_PushInput: file varname-mismatch.tmp, line 1
+# expect: Parse_PushInput: file variable-name-parentheses.tmp, line 1
+# expect: Parse_PushInput: file variable-name-parentheses.tmp, line 1
 
 # The guard condition must consist of only the guard variable, nothing else.
-INCS+=	ifndef-plus
-LINES.ifndef-plus= \
-	'.ifndef IFNDEF_PLUS && IFNDEF_SECOND' \
-	'IFNDEF_PLUS=' \
-	'IFNDEF_SECOND=' \
+INCS+=	variable-ifndef-plus
+LINES.variable-ifndef-plus= \
+	'.ifndef VARIABLE_IFNDEF_PLUS && VARIABLE_IFNDEF_SECOND' \
+	'VARIABLE_IFNDEF_PLUS=' \
+	'VARIABLE_IFNDEF_SECOND=' \
 	'.endif'
-# expect: Parse_PushInput: file ifndef-plus.tmp, line 1
-# expect: Parse_PushInput: file ifndef-plus.tmp, line 1
+# expect: Parse_PushInput: file variable-ifndef-plus.tmp, line 1
+# expect: Parse_PushInput: file variable-ifndef-plus.tmp, line 1
 
 # The guard condition must consist of only the guard variable, nothing else.
-INCS+=	if-plus
-LINES.if-plus= \
-	'.if !defined(IF_PLUS) && !defined(IF_SECOND)' \
-	'IF_PLUS=' \
-	'IF_SECOND=' \
+INCS+=	variable-if-plus
+LINES.variable-if-plus= \
+	'.if !defined(VARIABLE_IF_PLUS) && !defined(VARIABLE_IF_SECOND)' \
+	'VARIABLE_IF_PLUS=' \
+	'VARIABLE_IF_SECOND=' \
 	'.endif'
-# expect: Parse_PushInput: file if-plus.tmp, line 1
-# expect: Parse_PushInput: file if-plus.tmp, line 1
+# expect: Parse_PushInput: file variable-if-plus.tmp, line 1
+# expect: Parse_PushInput: file variable-if-plus.tmp, line 1
 
 # The variable name in an '.ifndef' guard must be given directly, it must not
 # contain any '$' expression.
-INCS+=	ifndef-indirect
-LINES.ifndef-indirect= \
-	'.ifndef $${IFNDEF_INDIRECT:L}' \
-	'IFNDEF_INDIRECT=' \
-	'.endif'
-# expect: Parse_PushInput: file ifndef-indirect.tmp, line 1
-# expect: Parse_PushInput: file ifndef-indirect.tmp, line 1
-
-# The variable name in an '.if' guard must be given directly, it must not contain
-# any '$' expression.
-INCS+=	if-indirect
-LINES.if-indirect= \
-	'.if !defined($${IF_INDIRECT:L})' \
-	'IF_INDIRECT=' \
+INCS+=	variable-ifndef-indirect
+LINES.variable-ifndef-indirect= \
+	'.ifndef $${VARIABLE_IFNDEF_INDIRECT:L}' \
+	'VARIABLE_IFNDEF_INDIRECT=' \
 	'.endif'
-# expect: Parse_PushInput: file if-indirect.tmp, line 1
-# expect: Parse_PushInput: file if-indirect.tmp, line 1
+# expect: Parse_PushInput: file variable-ifndef-indirect.tmp, line 1
+# expect: Parse_PushInput: file variable-ifndef-indirect.tmp, line 1
+
+# The variable name in an '.if' guard must be given directly, it must not
+# contain any '$' expression.
+INCS+=	variable-if-indirect
+LINES.variable-if-indirect= \
+	'.if !defined($${VARIABLE_IF_INDIRECT:L})' \
+	'VARIABLE_IF_INDIRECT=' \
+	'.endif'
+# expect: Parse_PushInput: file variable-if-indirect.tmp, line 1
+# 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
 # set anywhere, as long as it is set when the guarded file is included next.
-INCS+=	varassign-indirect
-LINES.varassign-indirect= \
-	'.ifndef VARASSIGN_INDIRECT' \
-	'$${VARASSIGN_INDIRECT:L}=' \
+INCS+=	variable-assign-indirect
+LINES.variable-assign-indirect= \
+	'.ifndef VARIABLE_ASSIGN_INDIRECT' \
+	'$${VARIABLE_ASSIGN_INDIRECT:L}=' \
 	'.endif'
-# expect: Parse_PushInput: file varassign-indirect.tmp, line 1
-# expect: Skipping 'varassign-indirect.tmp' because 'VARASSIGN_INDIRECT' is defined
+# 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.
-INCS+=	late-assignment
-LINES.late-assignment= \
-	'.ifndef LATE_ASSIGNMENT' \
-	'OTHER=' \
-	'LATE_ASSIGNMENT=' \
+INCS+=	variable-assign-late
+LINES.variable-assign-late= \
+	'.ifndef VARIABLE_ASSIGN_LATE' \
+	'VARIABLE_ASSIGN_LATE_OTHER=' \
+	'VARIABLE_ASSIGN_LATE=' \
 	'.endif'
-# expect: Parse_PushInput: file late-assignment.tmp, line 1
-# expect: Skipping 'late-assignment.tmp' because 'LATE_ASSIGNMENT' is defined
+# 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.
-INCS+=	two-conditions
-LINES.two-conditions= \
-	'.ifndef TWO_CONDITIONS' \
+INCS+=	variable-assign-nested
+LINES.variable-assign-nested= \
+	'.ifndef VARIABLE_ASSIGN_NESTED' \
 	'.  if 1' \
-	'TWO_CONDITIONS=' \
+	'.    for i in once' \
+	'VARIABLE_ASSIGN_NESTED=' \
+	'.    endfor' \
 	'.  endif' \
 	'.endif'
-# expect: Parse_PushInput: file two-conditions.tmp, line 1
-# expect: Skipping 'two-conditions.tmp' because 'TWO_CONDITIONS' is defined
+# expect: Parse_PushInput: file variable-assign-nested.tmp, line 1
+# 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.
-INCS+=	already-set
-LINES.already-set= \
-	'.ifndef ALREADY_SET' \
-	'ALREADY_SET=' \
-	'.endif'
-ALREADY_SET=
-# expect: Parse_PushInput: file already-set.tmp, line 1
-# expect: Parse_PushInput: file already-set.tmp, line 1
+# time, the file is not considered guarded.  This behavior is not finally
+# decided yet, as it is only a consequence of the current implementation.
+INCS+=	variable-already-defined
+LINES.variable-already-defined= \
+	'.ifndef VARIABLE_ALREADY_DEFINED' \
+	'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
 
 # The whole file content must be guarded by a single '.if' conditional, not by
-# several, even if they have the same effect.
-INCS+=	twice
-LINES.twice= \
-	'.ifndef TWICE_FIRST' \
-	'TWICE_FIRST=' \
+# 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.
+INCS+=	variable-two-times
+LINES.variable-two-times= \
+	'.ifndef VARIABLE_TWO_TIMES_1' \
+	'VARIABLE_TWO_TIMES_1=' \
 	'.endif' \
-	'.ifndef TWICE_SECOND' \
-	'TWICE_SECOND=' \
+	'.ifndef VARIABLE_TWO_TIMES_2' \
+	'VARIABLE_TWO_TIMES_2=' \
 	'.endif'
-# expect: Parse_PushInput: file twice.tmp, line 1
-# expect: Parse_PushInput: file twice.tmp, line 1
+# expect: Parse_PushInput: file variable-two-times.tmp, line 1
+# expect: Parse_PushInput: file variable-two-times.tmp, line 1
 
-# When multiple files use the same guard variable name, they exclude each
-# other.  It's the responsibility of the makefile authors to choose unique
-# variable names.  Typical choices are ${PROJECT}_${DIR}_${FILE}_MK.  This is
-# the same situation as in the 'already-set' test, and the file is not
-# considered guarded.
-INCS+=	reuse
-LINES.reuse= \
-	${LINES.guarded-if}
-# expect: Parse_PushInput: file reuse.tmp, line 1
-# expect: Parse_PushInput: file reuse.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.
+# 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 '_'.
+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
 
 # The conditional must come before the assignment, otherwise the conditional
 # is useless, as it always evaluates to false.
-INCS+=	swapped
-LINES.swapped= \
+INCS+=	variable-swapped
+LINES.variable-swapped= \
 	'SWAPPED=' \
 	'.ifndef SWAPPED' \
+	'.  error' \
 	'.endif'
-# expect: Parse_PushInput: file swapped.tmp, line 1
-# expect: Parse_PushInput: file swapped.tmp, line 1
+# expect: Parse_PushInput: file variable-swapped.tmp, line 1
+# expect: Parse_PushInput: file variable-swapped.tmp, line 1
 
-# If the guard variable is undefined at some later point, the guarded file is
-# included again.
-INCS+=	undef-between
-LINES.undef-between= \
-	'.ifndef UNDEF_BETWEEN' \
-	'UNDEF_BETWEEN=' \
-	'.endif'
-# expect: Parse_PushInput: file undef-between.tmp, line 1
-# expect: Parse_PushInput: file undef-between.tmp, line 1
-
-# If the guarded file undefines the guard variable, the guarded file is
-# included again.
-INCS+=	undef-inside
-LINES.undef-inside= \
-	'.ifndef UNDEF_INSIDE' \
-	'UNDEF_INSIDE=' \
-	'.undef UNDEF_INSIDE' \
+# If the guard variable is undefined between the first and the second time the
+# file is included, the guarded file is included again.
+INCS+=	variable-undef-between
+LINES.variable-undef-between= \
+	'.ifndef VARIABLE_UNDEF_BETWEEN' \
+	'VARIABLE_UNDEF_BETWEEN=' \
+	'.endif'
+UNDEF_BETWEEN.variable-undef-between= \
+	VARIABLE_UNDEF_BETWEEN
+# expect: Parse_PushInput: file variable-undef-between.tmp, line 1
+# expect: Parse_PushInput: file variable-undef-between.tmp, line 1
+
+# If the guard variable is undefined while the file is included the first
+# time, the guard does not have an effect, and the file is included again.
+INCS+=	variable-undef-inside
+LINES.variable-undef-inside= \
+	'.ifndef VARIABLE_UNDEF_INSIDE' \
+	'VARIABLE_UNDEF_INSIDE=' \
+	'.undef VARIABLE_UNDEF_INSIDE' \
+	'.endif'
+# expect: Parse_PushInput: file variable-undef-inside.tmp, line 1
+# expect: Parse_PushInput: file variable-undef-inside.tmp, line 1
+
+# If the file does not define the guard variable, the guard does not have an
+# effect, and the file is included again.
+INCS+=	variable-not-defined
+LINES.variable-not-defined= \
+	'.ifndef VARIABLE_NOT_DEFINED' \
 	'.endif'
-# expect: Parse_PushInput: file undef-inside.tmp, line 1
-# expect: Parse_PushInput: file undef-inside.tmp, line 1
+# expect: Parse_PushInput: file variable-not-defined.tmp, line 1
+# expect: Parse_PushInput: file variable-not-defined.tmp, line 1
 
 # The outermost '.if' must not have an '.elif' branch.
 INCS+=	if-elif
@@ -243,7 +304,8 @@ LINES.if-else = \
 # expect: Parse_PushInput: file if-else.tmp, line 1
 # expect: Parse_PushInput: file if-else.tmp, line 1
 
-# The inner '.if' directives may have an '.elif' or '.else'.
+# 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 = \
 	'.ifndef INNER_IF_ELIF_ELSE' \
@@ -264,35 +326,50 @@ LINES.inner-if-elif-else = \
 # expect: Parse_PushInput: file inner-if-elif-else.tmp, line 1
 # expect: Skipping 'inner-if-elif-else.tmp' because 'INNER_IF_ELIF_ELSE' is defined
 
-# The guard can not only be a variable, it can also be a target.
+# The guard can also be a target instead of a variable.  Using a target as a
+# guard has the benefit that a target cannot be undefined once it is defined.
+# The target should be declared '.NOTMAIN'.  Since the target names are
+# usually chosen according to a pattern that doesn't interfere with real
+# target names, they don't need to be declared '.PHONY' as they don't generate
+# filesystem operations.
 INCS+=	target
 LINES.target= \
 	'.if !target(__target.tmp__)' \
-	'__target.tmp__: .PHONY' \
+	'__target.tmp__: .NOTMAIN' \
 	'.endif'
 # expect: Parse_PushInput: file target.tmp, line 1
 # expect: Skipping 'target.tmp' because '__target.tmp__' is defined
 
-# When used for system files, the target name may include '<' and '>'.
+# When used for system files, the target name may include '<' and '>', for
+# symmetry with the '.include <sys.mk>' directive.  The characters '<' and '>'
+# are ordinary characters.
 INCS+=	target-sys
 LINES.target-sys= \
 	'.if !target(__<target-sys.tmp>__)' \
-	'__<target-sys.tmp>__: .PHONY' \
+	'__<target-sys.tmp>__: .NOTMAIN' \
 	'.endif'
 # expect: Parse_PushInput: file target-sys.tmp, line 1
 # expect: Skipping 'target-sys.tmp' because '__<target-sys.tmp>__' is defined
 
-# The target name may include variable references - which will be expanded.
+# The target name may include variable references.  These references are
+# expanded as usual.  Due to the current implementation, the expressions are
+# evaluated twice:  Once for checking whether the condition evaluates to true,
+# and once for determining the guard name.  This double evaluation should not
+# matter in practice, as guard expressions are expected to be simple,
+# deterministic and without side effects.
 INCS+=	target-indirect
 LINES.target-indirect= \
 	'.if !target($${target-indirect.tmp:L})' \
-	'target-indirect.tmp: .PHONY' \
+	'target-indirect.tmp: .NOTMAIN' \
 	'.endif'
 # expect: Parse_PushInput: file target-indirect.tmp, line 1
 # expect: Skipping 'target-indirect.tmp' because 'target-indirect.tmp' is defined
 
-# A common form of guard target is __${.PARSEFILE}__.
-# This is only useful of course if basename is unique.
+# A common form of guard target is __${.PARSEFILE}__.  This form can only be
+# used if all files using this form have unique basenames.  To get a robust
+# pattern based on the same idea, use __${.PARSEDIR}/${.PARSEFILE}__ instead.
+# This form does not work when the basename contains whitespace characters, as
+# it is not possible to define a target with whitespace, not even by cheating.
 INCS+=	target-indirect-PARSEFILE
 LINES.target-indirect-PARSEFILE= \
 	'.if !target(__$${.PARSEFILE}__)' \
@@ -301,8 +378,8 @@ LINES.target-indirect-PARSEFILE= \
 # expect: Parse_PushInput: file target-indirect-PARSEFILE.tmp, line 1
 # expect: Skipping 'target-indirect-PARSEFILE.tmp' because '__target-indirect-PARSEFILE.tmp__' is defined
 
-# Confirm that two such guards do not conflict
-# again, assuming the basenames are unique.
+# Two files with different basenames can both use the same syntactic pattern
+# for the target guard name, as the expressions expand to different strings.
 INCS+=	target-indirect-PARSEFILE2
 LINES.target-indirect-PARSEFILE2= \
 	'.if !target(__$${.PARSEFILE}__)' \
@@ -311,7 +388,22 @@ LINES.target-indirect-PARSEFILE2= \
 # expect: Parse_PushInput: file target-indirect-PARSEFILE2.tmp, line 1
 # expect: Skipping 'target-indirect-PARSEFILE2.tmp' because '__target-indirect-PARSEFILE2.tmp__' is defined
 
-# Another common form of guard target is __${.PARSEFILE:tA}__.
+# 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.
+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
+
+# 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
+# for files from other directories, as the modifier ':tA' resolves a file
+# relative to the current working directory ('.OBJDIR').  To get a robust
+# pattern, use __${.PARSEDIR}/.${.PARSEFILE}__ instead.
 INCS+=	target-indirect-PARSEFILE-tA
 LINES.target-indirect-PARSEFILE-tA= \
 	'.if !target(__$${.PARSEFILE:tA}__)' \
@@ -319,11 +411,61 @@ LINES.target-indirect-PARSEFILE-tA= \
 	'.endif'
 # expect: Parse_PushInput: file target-indirect-PARSEFILE-tA.tmp, line 1
 # expect: Skipping 'target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
-# The actual target is __${.OBJDIR}/target-indirect-PARSEFILE-tA.tmp__ but
-# ${.OBJDIR}/ gets stripped in post processing.
+# The actual target starts with '__${.OBJDIR}/', see the .rawout file, but the
+# string '${.OBJDIR}/' gets stripped in post processing.
 
-# If the target is not defined when including the file the next time, the file
-# is not guarded.
+# Using the ':tA' modifier to construct guard target names is generally wrong,
+# as the ':tA' modifier only works for files in the current working directory.
+# For files from subdirectories that are not also found in the current working
+# directory, applying the modifier ':tA' has no effect.
+INCS+=	subdir/target-indirect-PARSEFILE-tA
+LINES.subdir/target-indirect-PARSEFILE-tA= \
+	'.if !target(__$${.PARSEFILE:tA}__)' \
+	'__$${.PARSEFILE:tA}__: .NOTMAIN' \
+	'.endif'
+# expect: Parse_PushInput: file subdir/target-indirect-PARSEFILE-tA.tmp, line 1
+# expect: Skipping 'subdir/target-indirect-PARSEFILE-tA.tmp' because '__target-indirect-PARSEFILE-tA.tmp__' is defined
+# The guard target name does not include any directory since the ':tA'
+# modifier file cannot resolve the file in the current working directory.
+
+# If there are two subdirectories that both have a file with the same basename
+# that uses '${.PARSEFILE:tA}' as its guard target, the second file reuses the
+# guard name from the first file.  To get a robust scheme of guard target
+# names, use __${.PARSEDIR}/.${.PARSEFILE}__ instead.
+INCS+=	subdir2/target-indirect-PARSEFILE-tA
+LINES.subdir2/target-indirect-PARSEFILE-tA= \
+	'.if !target(__$${.PARSEFILE:tA}__)' \
+	'__$${.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
+
+# Using both '.PARSEDIR' and '.PARSEFILE' to form the guard target name is a
+# robust approach.
+INCS+=	target-indirect-PARSEDIR-PARSEFILE
+LINES.target-indirect-PARSEDIR-PARSEFILE= \
+	'.if !target(__$${.PARSEDIR}/$${.PARSEFILE}__)' \
+	'__$${.PARSEDIR}/$${.PARSEFILE}__: .NOTMAIN' \
+	'.endif'
+# expect: Parse_PushInput: file target-indirect-PARSEDIR-PARSEFILE.tmp, line 1
+# expect: Skipping 'target-indirect-PARSEDIR-PARSEFILE.tmp' because '__target-indirect-PARSEDIR-PARSEFILE.tmp__' is defined
+# The actual target starts with '__${.OBJDIR}/', see the .rawout file, but the
+# string '${.OBJDIR}/' gets stripped in post processing.
+
+# Using the combination of '.PARSEDIR' and '.PARSEFILE', a file in a
+# subdirectory gets a different guard target name than the previous one.
+INCS+=	subdir/target-indirect-PARSEDIR-PARSEFILE
+LINES.subdir/target-indirect-PARSEDIR-PARSEFILE= \
+	'.if !target(__$${.PARSEDIR}/$${.PARSEFILE}__)' \
+	'__$${.PARSEDIR}/$${.PARSEFILE}__: .NOTMAIN' \
+	'.endif'
+# expect: Parse_PushInput: file subdir/target-indirect-PARSEDIR-PARSEFILE.tmp, line 1
+# expect: Skipping 'subdir/target-indirect-PARSEDIR-PARSEFILE.tmp' because '__subdir/target-indirect-PARSEDIR-PARSEFILE.tmp__' is defined
+# The actual target starts with '__${.OBJDIR}/', see the .rawout file, but the
+# string '${.OBJDIR}/' gets stripped in post processing.
+
+# If the guard target is not defined when including the file the next time,
+# the file is processed again.
 INCS+=	target-unguarded
 LINES.target-unguarded= \
 	'.if !target(target-unguarded)' \
@@ -335,38 +477,55 @@ LINES.target-unguarded= \
 INCS+=	target-plus
 LINES.target-plus= \
 	'.if !target(target-plus) && 1' \
-	'target-plus: .PHONY' \
+	'target-plus: .NOTMAIN' \
 	'.endif'
 # expect: Parse_PushInput: file target-plus.tmp, line 1
 # expect: Parse_PushInput: file target-plus.tmp, line 1
 
-# If the guard target is defined before the file is included for the first
-# time, the file is not considered guarded.
+# 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: .PHONY' \
+	'target-already-set: .NOTMAIN' \
 	'.endif'
-target-already-set: .PHONY
+target-already-set: .NOTMAIN
 # expect: Parse_PushInput: file target-already-set.tmp, line 1
 # expect: Parse_PushInput: file target-already-set.tmp, line 1
 
-
-# Include each of the files twice.  The directive-include-guard.exp file
-# contains a single entry for the files whose multiple-inclusion guard works,
-# and two entries for the files that are not protected against multiple
-# inclusion.
+# A target name cannot contain the character '!'.  In the condition, the '!'
+# is syntactically valid, but in the dependency declaration line, the '!' is
+# interpreted as the '!' dependency operator, no matter whether it occurs at
+# the beginning or in the middle of a target name.  Escaping it as '${:U!}'
+# doesn't work, as the whole line is first expanded and then scanned for the
+# dependency operator.  Escaping it as '\!' doesn't work either, even though
+# the '\' escapes the '!' from being a dependency operator, but when reading
+# the target name, the '\' is kept, resulting in the target name
+# '\!target-name-exclamation' instead of '!target-name-exclamation'.
+INCS+=	target-name-exclamation
+LINES.target-name-exclamation= \
+	'.if !target(!target-name-exclamation)' \
+	'\!target-name-exclamation: .NOTMAIN' \
+	'.endif'
+# 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.
 #
 # Some debug output lines are suppressed in the .exp file, see ./Makefile.
 .for i in ${INCS}
 .  for fname in $i.tmp
+_:=	${fname:H:N.:@dir@${:!mkdir -p ${dir}!}@}
 _!=	printf '%s\n' ${LINES.$i} > ${fname}
 .MAKEFLAGS: -dp
 .include "${.CURDIR}/${fname}"
-.undef ${i:Mundef-between:%=UNDEF_BETWEEN}
+.undef ${UNDEF_BETWEEN.$i:U}
 .include "${.CURDIR}/${fname}"
 .MAKEFLAGS: -d0
 _!=	rm ${fname}
+_:=	${fname:H:N.:@dir@${:!rmdir ${dir}!}@}
 .  endfor
 .endfor
 

Reply via email to