[src/trunk]: src/usr.bin/make make: skip a file protected by a multiple-inclu...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/5e99faa2e7ba
branches:  trunk
changeset: 376506:5e99faa2e7ba
user:      rillig <rillig%NetBSD.org@localhost>
date:      Wed Jun 21 14:33:36 2023 +0000

description:
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.

diffstat:

 usr.bin/make/parse.c                                |    6 +-
 usr.bin/make/unit-tests/directive-include-guard.exp |   22 ++-
 usr.bin/make/unit-tests/directive-include-guard.mk  |  117 +++++++++++++++----
 3 files changed, 108 insertions(+), 37 deletions(-)

diffs (truncated from 348 to 300 lines):

diff -r bb41cd240e2b -r 5e99faa2e7ba usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Wed Jun 21 12:27:50 2023 +0000
+++ b/usr.bin/make/parse.c      Wed Jun 21 14:33:36 2023 +0000
@@ -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;
diff -r bb41cd240e2b -r 5e99faa2e7ba usr.bin/make/unit-tests/directive-include-guard.exp
--- a/usr.bin/make/unit-tests/directive-include-guard.exp       Wed Jun 21 12:27:50 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-include-guard.exp       Wed Jun 21 14:33:36 2023 +0000
@@ -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
diff -r bb41cd240e2b -r 5e99faa2e7ba usr.bin/make/unit-tests/directive-include-guard.mk
--- a/usr.bin/make/unit-tests/directive-include-guard.mk        Wed Jun 21 12:27:50 2023 +0000
+++ b/usr.bin/make/unit-tests/directive-include-guard.mk        Wed Jun 21 14:33:36 2023 +0000
@@ -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