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.