Module Name:    src
Committed By:   rillig
Date:           Sun Dec 10 19:56:53 UTC 2023

Modified Files:
        src/usr.bin/make: var.c

Log Message:
make: clean up comments and local identifiers

No binary change, except for line numbers in assertions.


To generate a diff of this commit:
cvs rdiff -u -r1.1078 -r1.1079 src/usr.bin/make/var.c

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/var.c
diff -u src/usr.bin/make/var.c:1.1078 src/usr.bin/make/var.c:1.1079
--- src/usr.bin/make/var.c:1.1078	Sun Dec 10 18:59:50 2023
+++ src/usr.bin/make/var.c	Sun Dec 10 19:56:53 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: var.c,v 1.1078 2023/12/10 18:59:50 rillig Exp $	*/
+/*	$NetBSD: var.c,v 1.1079 2023/12/10 19:56:53 rillig Exp $	*/
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -139,7 +139,7 @@
 #include "metachar.h"
 
 /*	"@(#)var.c	8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: var.c,v 1.1078 2023/12/10 18:59:50 rillig Exp $");
+MAKE_RCSID("$NetBSD: var.c,v 1.1079 2023/12/10 19:56:53 rillig Exp $");
 
 /*
  * Variables are defined using one of the VAR=value assignments.  Their
@@ -378,11 +378,11 @@ CanonicalVarname(Substring name)
 	if (Substring_Equals(name, ".TARGET"))
 		return Substring_InitStr(TARGET);
 
+	/* GNU make has an additional alias $^ == ${.ALLSRC}. */
+
 	if (Substring_Equals(name, ".SHELL") && shellPath == NULL)
 		Shell_Init();
 
-	/* GNU make has an additional alias $^ == ${.ALLSRC}. */
-
 	return name;
 }
 
@@ -911,7 +911,7 @@ Var_UnExport(bool isEnv, const char *arg
 
 /*
  * When there is a variable of the same name in the command line scope, the
- * global variable would not be visible anywhere.  Therefore there is no
+ * global variable would not be visible anywhere.  Therefore, there is no
  * point in setting it at all.
  *
  * See 'scope == SCOPE_CMDLINE' in Var_SetWithFlags.
@@ -969,7 +969,7 @@ Var_SetWithFlags(GNode *scope, const cha
 			Var_Delete(SCOPE_GLOBAL, name);
 		}
 		if (strcmp(name, ".SUFFIXES") == 0) {
-			/* special: treat as readOnly */
+			/* special: treat as read-only */
 			DEBUG3(VAR, "%s: %s = %s ignored (read-only)\n",
 			    scope->name, name, val);
 			return;
@@ -990,12 +990,14 @@ Var_SetWithFlags(GNode *scope, const cha
 			ExportVar(name, VEM_PLAIN);
 	}
 
-	/*
-	 * Any variables given on the command line are automatically exported
-	 * to the environment (as per POSIX standard), except for internals.
-	 */
 	if (scope == SCOPE_CMDLINE) {
 		v->fromCmd = true;
+
+		/*
+		 * Any variables given on the command line are automatically
+		 * exported to the environment (as per POSIX standard), except
+		 * for internals.
+		 */
 		if (!(flags & VAR_SET_NO_EXPORT) && name[0] != '.') {
 
 			/*
@@ -1030,15 +1032,8 @@ Var_Set(GNode *scope, const char *name, 
 }
 
 /*
- * Set the variable name to the value val in the given scope.
- *
- * If the variable doesn't yet exist, it is created.
- * Otherwise the new value overwrites and replaces the old value.
- *
- * Input:
- *	scope		scope in which to set it
- *	name		name of the variable to set, is expanded once
- *	val		value to give to the variable
+ * In the scope, expand the variable name once, then create the variable or
+ * replace its value.
  */
 void
 Var_SetExpand(GNode *scope, const char *name, const char *val)
@@ -1445,14 +1440,6 @@ ModifyWord_SysVSubst(Substring word, Sep
 }
 #endif
 
-
-struct ModifyWord_SubstArgs {
-	Substring lhs;
-	Substring rhs;
-	PatternFlags pflags;
-	bool matched;
-};
-
 static const char *
 Substring_Find(Substring haystack, Substring needle)
 {
@@ -1466,6 +1453,13 @@ Substring_Find(Substring haystack, Subst
 	return NULL;
 }
 
+struct ModifyWord_SubstArgs {
+	Substring lhs;
+	Substring rhs;
+	PatternFlags pflags;
+	bool matched;
+};
+
 /*
  * Callback for ModifyWords to implement the :S,from,to, modifier.
  * Perform a string substitution on the given word.
@@ -1915,8 +1909,8 @@ FormatTime(const char *fmt, time_t t, bo
  * XXX: As of 2020-11-15, some modifiers such as :S, :C, :P, :L do not
  * need to be followed by a ':' or endc; this was an unintended mistake.
  *
- * If parsing fails because of a missing delimiter (as in the :S, :C or :@
- * modifiers), return AMR_CLEANUP.
+ * If parsing fails because of a missing delimiter after a modifier part (as
+ * in the :S, :C or :@ modifiers), return AMR_CLEANUP.
  *
  * If parsing fails because the modifier is unknown, return AMR_UNKNOWN to
  * try the SysV modifier ${VAR:from=to} as fallback.  This should only be
@@ -3078,8 +3072,8 @@ ApplyModifier_ToSep(const char **pp, Mod
 	const char *sep = *pp + 2;
 
 	/*
-	 * Even in parse-only mode, proceed as normal since there is
-	 * neither any observable side effect nor a performance penalty.
+	 * Even in parse-only mode, apply the side effects, since the side
+	 * effects are neither observable nor is there a performance penalty.
 	 * Checking for wantRes for every single piece of code in here
 	 * would make the code in this function too hard to read.
 	 */
@@ -3236,14 +3230,14 @@ ApplyModifier_Words(const char **pp, Mod
 	Expr *expr = ch->expr;
 	int first, last;
 	const char *p;
-	LazyBuf estrBuf;
-	FStr festr;
+	LazyBuf argBuf;
+	FStr arg;
 
 	(*pp)++;		/* skip the '[' */
-	if (!ParseModifierPart(pp, ']', expr->emode, ch, &estrBuf))
+	if (!ParseModifierPart(pp, ']', expr->emode, ch, &argBuf))
 		return AMR_CLEANUP;
-	festr = LazyBuf_DoneGet(&estrBuf);
-	p = festr.str;
+	arg = LazyBuf_DoneGet(&argBuf);
+	p = arg.str;
 
 	if (!IsDelimiter(**pp, ch))
 		goto bad_modifier;		/* Found junk after ']' */
@@ -3308,11 +3302,11 @@ ApplyModifier_Words(const char **pp, Mod
 		ch->sep, ch->oneBigWord));
 
 ok:
-	FStr_Done(&festr);
+	FStr_Done(&arg);
 	return AMR_OK;
 
 bad_modifier:
-	FStr_Done(&festr);
+	FStr_Done(&arg);
 	return AMR_BAD;
 }
 
@@ -4047,11 +4041,11 @@ ApplySingleModifier(const char **pp, Mod
 }
 
 #if __STDC_VERSION__ >= 199901L
-#define ModChain_Literal(expr, startc, endc, sep, oneBigWord) \
+#define ModChain_Init(expr, startc, endc, sep, oneBigWord) \
 	(ModChain) { expr, startc, endc, sep, oneBigWord }
 #else
 MAKE_INLINE ModChain
-ModChain_Literal(Expr *expr, char startc, char endc, char sep, bool oneBigWord)
+ModChain_Init(Expr *expr, char startc, char endc, char sep, bool oneBigWord)
 {
 	ModChain ch;
 	ch.expr = expr;
@@ -4072,7 +4066,7 @@ ApplyModifiers(
     char endc		/* ')' or '}'; or '\0' for indirect modifiers */
 )
 {
-	ModChain ch = ModChain_Literal(expr, startc, endc, ' ', false);
+	ModChain ch = ModChain_Init(expr, startc, endc, ' ', false);
 	const char *p;
 	const char *mod;
 
@@ -4093,17 +4087,17 @@ ApplyModifiers(
 		ApplyModifierResult res;
 
 		if (*p == '$') {
+			/*
+			 * TODO: Only evaluate the expression once, no matter
+			 * whether it's an indirect modifier or the initial
+			 * part of a SysV modifier.
+			 */
 			ApplyModifiersIndirectResult amir =
 			    ApplyModifiersIndirect(&ch, &p);
 			if (amir == AMIR_CONTINUE)
 				continue;
 			if (amir == AMIR_OUT)
 				break;
-			/*
-			 * It's neither '${VAR}:' nor '${VAR}}'.  Try to parse
-			 * it as a SysV modifier, as that is the only modifier
-			 * that can start with '$'.
-			 */
 		}
 
 		mod = p;
@@ -4120,7 +4114,7 @@ ApplyModifiers(
 	return;
 
 bad_modifier:
-	/* XXX: The modifier end is only guessed. */
+	/* Take a guess at where the modifier ends. */
 	Error("Bad modifier \":%.*s\" for variable \"%s\"",
 	    (int)strcspn(mod, ":)}"), mod, expr->name);
 
@@ -4221,7 +4215,6 @@ ParseVarname(const char **pp, char start
 		if (*p == endc)
 			depth--;
 
-		/* An expression inside an expression, expand. */
 		if (*p == '$') {
 			FStr nested_val = Var_Parse(&p, scope, emode);
 			/* TODO: handle errors */
@@ -4434,9 +4427,9 @@ ParseVarnameLong(
 		 * variable name, such as :L or :?.
 		 *
 		 * Most modifiers leave this expression in the "undefined"
-		 * state (VES_UNDEF), only a few modifiers like :D, :U, :L,
+		 * state (DEF_UNDEF), only a few modifiers like :D, :U, :L,
 		 * :P turn this undefined expression into a defined
-		 * expression (VES_DEF).
+		 * expression (DEF_DEFINED).
 		 *
 		 * In the end, after applying all modifiers, if the expression
 		 * is still undefined, Var_Parse will return an empty string
@@ -4457,12 +4450,12 @@ ParseVarnameLong(
 }
 
 #if __STDC_VERSION__ >= 199901L
-#define Expr_Literal(name, value, emode, scope, defined) \
-	{ name, value, emode, scope, defined }
+#define Expr_Init(name, value, emode, scope, defined) \
+	(Expr) { name, value, emode, scope, defined }
 #else
 MAKE_INLINE Expr
-Expr_Literal(const char *name, FStr value,
-	     VarEvalMode emode, GNode *scope, ExprDefined defined)
+Expr_Init(const char *name, FStr value,
+	  VarEvalMode emode, GNode *scope, ExprDefined defined)
 {
 	Expr expr;
 
@@ -4477,8 +4470,7 @@ Expr_Literal(const char *name, FStr valu
 
 /*
  * Expressions of the form ${:U...} with a trivial value are often generated
- * by .for loops and are boring, therefore parse and evaluate them in a fast
- * lane without debug logging.
+ * by .for loops and are boring, so evaluate them without debug logging.
  */
 static bool
 Var_Parse_FastLane(const char **pp, VarEvalMode emode, FStr *out_value)
@@ -4505,18 +4497,16 @@ Var_Parse_FastLane(const char **pp, VarE
 }
 
 /*
- * Given the start of an expression (such as $v, $(VAR),
- * ${VAR:Mpattern}), extract the variable name and value, and the modifiers,
- * if any.  While doing that, apply the modifiers to the value of the
- * expression, forming its final value.  A few of the modifiers such as :!cmd!
- * or ::= have side effects.
+ * Given the start of an expression (such as $v, $(VAR), ${VAR:Mpattern}),
+ * extract the variable name and the modifiers, if any.  While parsing, apply
+ * the modifiers to the value of the expression.
  *
  * Input:
  *	*pp		The string to parse.
  *			When called from CondParser_FuncCallEmpty, it can
  *			also point to the "y" of "empty(VARNAME:Modifiers)".
- *	scope		The scope for finding variables
- *	emode		Controls the exact details of parsing and evaluation
+ *	scope		The scope for finding variables.
+ *	emode		Controls the exact details of parsing and evaluation.
  *
  * Output:
  *	*pp		The position where to continue parsing.
@@ -4557,15 +4547,13 @@ Var_Parse(const char **pp, GNode *scope,
 	bool dynamic;
 	const char *extramodifiers;
 	Var *v;
-	Expr expr = Expr_Literal(NULL, FStr_InitRefer(NULL), emode,
+	Expr expr = Expr_Init(NULL, FStr_InitRefer(NULL), emode,
 	    scope, DEF_REGULAR);
 	FStr val;
 
 	if (Var_Parse_FastLane(pp, emode, &val))
 		return val;
 
-	/* TODO: Reduce computations in parse-only mode. */
-
 	DEBUG2(VAR, "Var_Parse: %s (%s)\n", start, VarEvalMode_Name[emode]);
 
 	val = FStr_InitRefer(NULL);
@@ -4776,7 +4764,6 @@ Var_Subst(const char *str, GNode *scope,
 	 * Set true if an error has already been reported, to prevent a
 	 * plethora of messages when recursing
 	 */
-	/* See varparse-errors.mk for why the 'static' is necessary here. */
 	static bool errorReported;
 
 	Buf_Init(&res);

Reply via email to