[src/trunk]: src/usr.bin/make/unit-tests tests/make: proofread and clean up c...

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

details:   https://anonhg.NetBSD.org/src/rev/b239669d03dd
branches:  trunk
changeset: 1027553:b239669d03dd
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Dec 11 09:53:53 2021 +0000

description:
tests/make: proofread and clean up comments for function 'empty'

This prepares a refactoring for handling the function 'empty' in
conditionals like '.if'.

The function 'empty' is fundamentally different from all other functions
since it is parsed differently and passes its result on different path
than the other functions.  Splitting up these code paths will untangle
the control flow of parsing a condition like 'empty(VARNAME)'.  It will
also remove several ARGSUSED and MAKE_ATTR_UNUSED that make the current
code smell.

diffstat:

 usr.bin/make/unit-tests/cond-cmp-string.mk  |   4 +-
 usr.bin/make/unit-tests/cond-func-empty.exp |   4 +-
 usr.bin/make/unit-tests/cond-func-empty.mk  |  73 ++++++++++++++++------------
 3 files changed, 45 insertions(+), 36 deletions(-)

diffs (168 lines):

diff -r f195ae5bc1fd -r b239669d03dd usr.bin/make/unit-tests/cond-cmp-string.mk
--- a/usr.bin/make/unit-tests/cond-cmp-string.mk        Fri Dec 10 23:56:17 2021 +0000
+++ b/usr.bin/make/unit-tests/cond-cmp-string.mk        Sat Dec 11 09:53:53 2021 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: cond-cmp-string.mk,v 1.14 2021/01/19 19:54:57 rillig Exp $
+# $NetBSD: cond-cmp-string.mk,v 1.15 2021/12/11 09:53:53 rillig Exp $
 #
 # Tests for string comparisons in .if conditions.
 
@@ -26,7 +26,7 @@
 # starting point for variable expressions.  Applying the :U modifier to such
 # an undefined expression turns it into a defined expression.
 #
-# See ApplyModifier_Defined and VEF_DEF.
+# See ApplyModifier_Defined and DEF_DEFINED.
 .if ${:Ustr} != "str"
 .  error
 .endif
diff -r f195ae5bc1fd -r b239669d03dd usr.bin/make/unit-tests/cond-func-empty.exp
--- a/usr.bin/make/unit-tests/cond-func-empty.exp       Fri Dec 10 23:56:17 2021 +0000
+++ b/usr.bin/make/unit-tests/cond-func-empty.exp       Sat Dec 11 09:53:53 2021 +0000
@@ -1,5 +1,5 @@
-make: "cond-func-empty.mk" line 152: Unclosed variable "WORD"
-make: "cond-func-empty.mk" line 152: Malformed conditional (empty(WORD)
+make: "cond-func-empty.mk" line 149: Unclosed variable "WORD"
+make: "cond-func-empty.mk" line 149: Malformed conditional (empty(WORD)
 make: Fatal errors encountered -- cannot continue
 make: stopped in unit-tests
 exit status 1
diff -r f195ae5bc1fd -r b239669d03dd usr.bin/make/unit-tests/cond-func-empty.mk
--- a/usr.bin/make/unit-tests/cond-func-empty.mk        Fri Dec 10 23:56:17 2021 +0000
+++ b/usr.bin/make/unit-tests/cond-func-empty.mk        Sat Dec 11 09:53:53 2021 +0000
@@ -1,10 +1,10 @@
-# $NetBSD: cond-func-empty.mk,v 1.14 2021/04/11 13:35:56 rillig Exp $
+# $NetBSD: cond-func-empty.mk,v 1.15 2021/12/11 09:53:53 rillig Exp $
 #
 # Tests for the empty() function in .if conditions, which tests a variable
 # expression for emptiness.
 #
-# Note that the argument in the parentheses is indeed a variable name,
-# optionally followed by variable modifiers.
+# Note that the argument in the parentheses is a variable name, not a variable
+# expression, optionally followed by variable modifiers.
 #
 
 .undef UNDEF
@@ -25,14 +25,10 @@
 .endif
 
 # The :S modifier replaces the empty value with an actual word.  The
-# expression is now no longer empty, but it is still possible to see whether
-# the expression was based on an undefined variable.  The expression has the
-# flag VEF_UNDEF.
-#
-# The expression does not have the flag VEF_DEF though, therefore it is still
-# considered undefined.  Yes, indeed, undefined but not empty.  There are a
-# few variable modifiers that turn an undefined expression into a defined
-# expression, among them :U and :D, but not :S.
+# expression is now no longer empty, but it is still based on an undefined
+# variable (DEF_UNDEF).  There are a few variable modifiers that turn an
+# undefined expression into a defined expression, among them :U and :D, but
+# not :S.
 #
 # XXX: This is hard to explain to someone who doesn't know these
 # implementation details.
@@ -41,19 +37,19 @@
 .  error
 .endif
 
-# The :U modifier modifies expressions based on undefined variables
-# (DEF_UNDEF) by adding the DEF_DEFINED flag, which marks the expression
-# as "being interesting enough to be further processed".
+# The :U modifier changes the state of a previously undefined expression from
+# DEF_UNDEF to DEF_DEFINED.  This marks the expression as "being interesting
+# enough to be further processed".
 #
 .if empty(UNDEF:S,^$,value,W:Ufallback)
 .  error
 .endif
 
 # And now to the surprising part.  Applying the following :S modifier to the
-# undefined expression makes it non-empty, but the marker VEF_UNDEF is
-# preserved nevertheless.  The :U modifier that follows only looks at the
-# VEF_UNDEF flag to decide whether the variable is defined or not.  This kind
-# of makes sense since the :U modifier tests the _variable_, not the
+# undefined expression makes it non-empty, but the expression is still in
+# state DEF_UNDEF.  The :U modifier that follows only looks at the state
+# DEF_UNDEF to decide whether the variable is defined or not.  This kind of
+# makes sense since the :U modifier tests the _variable_, not the
 # _expression_.
 #
 # But since the variable was undefined to begin with, the fallback value from
@@ -78,12 +74,13 @@
 .  error
 .endif
 
-# The empty variable named "" gets a fallback value of " ", which counts as
-# empty.
+# The following example constructs an expression with the variable name ""
+# and the value " ".  This expression counts as empty since the value contains
+# only whitespace.
 #
 # Contrary to the other functions in conditionals, the trailing space is not
 # stripped off, as can be seen in the -dv debug log.  If the space had been
-# stripped, it wouldn't make a difference in this case.
+# stripped, it wouldn't make a difference in this case, but in other cases.
 #
 .if !empty(:U )
 .  error
@@ -93,8 +90,8 @@
 # neither leading nor trailing spaces are trimmed in the argument of the
 # function.  If the spaces were trimmed, the variable name would be "" and
 # that variable is indeed undefined.  Since ParseEmptyArg calls Var_Parse
-# without VARE_UNDEFERR, the value of the undefined variable is
-# returned as an empty string.
+# without VARE_UNDEFERR, the value of the undefined variable is returned as an
+# empty string.
 ${:U }=        space
 .if empty( )
 .  error
@@ -129,8 +126,8 @@
 #
 # If everything goes well, the argument expands to "WORD", and that variable
 # is defined at the beginning of this file.  The surrounding 'W' and 'D'
-# ensure that the parser in ParseEmptyArg has the correct position, both
-# before and after the call to Var_Parse.
+# ensure that ParseEmptyArg keeps track of the parsing position, both before
+# and after the call to Var_Parse.
 .if empty(W${:UOR}D)
 .  error
 .endif
@@ -155,17 +152,29 @@
 .  error
 .endif
 
-# Between 2020-06-28 and var.c 1.226 from 2020-07-02, this paragraph generated
-# a wrong error message "Variable VARNAME is recursive".
+# Since cond.c 1.76 from 2020-06-28 and before var.c 1.226 from 2020-07-02,
+# the following example generated a wrong error message "Variable VARNAME is
+# recursive".
 #
-# The bug was that the !empty() condition was evaluated, even though this was
-# not necessary since the defined() condition already evaluated to false.
+# Since at least 1993, the manual page claimed that irrelevant parts of
+# conditions were not evaluated, but that was wrong for a long time.  The
+# expressions in irrelevant parts of the condition were actually evaluated,
+# they just allowed undefined variables to be used in the conditions, and the
+# result of evaluating them was not used further.  These unnecessary
+# evaluations were fixed in several commits, starting with var.c 1.226 from
+# 2020-07-02.
+#
+# In this example, the variable "VARNAME2" is not defined, so evaluation of
+# the condition should have stopped at this point, and the rest of the
+# condition should have been processed in parse-only mode.  The right-hand
+# side containing the '!empty' was evaluated though, as it had always been.
 #
 # When evaluating the !empty condition, the variable name was parsed as
 # "VARNAME${:U2}", but without expanding any nested variable expression, in
-# this case the ${:U2}.  Therefore, the variable name came out as simply
-# "VARNAME".  Since this variable name should have been discarded quickly after
-# parsing it, this unrealistic variable name should have done no harm.
+# this case the ${:U2}.  The expression '${:U2}' was replaced with an empty
+# string, the resulting variable name was thus "VARNAME".  This conceptually
+# wrong variable name should have been discarded quickly after parsing it, to
+# prevent it from doing any harm.
 #
 # The variable expression was expanded though, and this was wrong.  The
 # expansion was done without VARE_WANTRES (called VARF_WANTRES back