TIP 582: Comments in Expressions

6 files changed, 162 insertions, 6 deletions

diff --git a/doc/expr.n b/doc/expr.n
index 69f6399..2c2742c 100644
--- a/doc/expr.n
+++ b/doc/expr.n
@@ -41,6 +41,12 @@ When an expression evaluates to an integer, the value is the decimal form of
 the integer, and when an expression evaluates to a floating-point number, the
 value is the form produced by the \fB%g\fR format specifier of Tcl's
 \fBformat\fR command.
+.PP
+.VS "TIP 582"
+You can use \fB#\fR at any point in the expression (except inside double
+quotes or braces) to start a comment. Comments last to the end of the line or
+the end of the expression, whichever comes first.
+.VE "TIP 582"
 .SS OPERANDS
 .PP
 An expression consists of a combination of operands, operators, parentheses and
@@ -487,7 +493,9 @@ value of true:
 .PP
 .CS
 set isTrue [\fBexpr\fR {
+    # Does the environment variable exist, and...
     [info exists ::env(SOME_ENV_VAR)] &&
+    # ...does it contain a proper true value?
     [string is true -strict $::env(SOME_ENV_VAR)]
 }]
 .CE
diff --git a/generic/tclCompExpr.c b/generic/tclCompExpr.c
index aabd764..41938e3 100644
--- a/generic/tclCompExpr.c
+++ b/generic/tclCompExpr.c
@@ -164,6 +164,8 @@ enum Marks {
 				 * "=" is encountered.  */
 #define INVALID		5	/* A parse error. Used when any punctuation
 				 * appears that's not a supported operator. */
+#define COMMENT		6	/* Comment. Lasts to end of line or end of
+				 * expression, whichever comes first. */
 
 /* Leaf lexemes */
 
@@ -462,7 +464,7 @@ static const unsigned char Lexeme[] = {
 	INVALID		/* FS */,	INVALID		/* GS */,
 	INVALID		/* RS */,	INVALID		/* US */,
 	INVALID		/* SPACE */,	0		/* ! or != */,
-	QUOTED		/* " */,	INVALID		/* # */,
+	QUOTED		/* " */,	0		/* # */,
 	VARIABLE	/* $ */,	MOD		/* % */,
 	0		/* & or && */,	INVALID		/* ' */,
 	OPEN_PAREN	/* ( */,	CLOSE_PAREN	/* ) */,
@@ -674,9 +676,10 @@ ParseExpr(
 	    OpNode *newPtr = NULL;
 
 	    do {
-	      if (size <= UINT_MAX/sizeof(OpNode)) {
-		newPtr = (OpNode *)attemptckrealloc(nodes, size * sizeof(OpNode));
-	      }
+		if (size <= UINT_MAX/sizeof(OpNode)) {
+		    newPtr = (OpNode *) attemptckrealloc(nodes,
+			    size * sizeof(OpNode));
+		}
 	    } while ((newPtr == NULL)
 		    && ((size -= (size - nodesUsed) / 2) > nodesUsed));
 	    if (newPtr == NULL) {
@@ -708,6 +711,10 @@ ParseExpr(
 	    int b;
 
 	    switch (lexeme) {
+	    case COMMENT:
+		start += scanned;
+		numBytes -= scanned;
+		continue;
 	    case INVALID:
 		msg = Tcl_ObjPrintf("invalid character \"%.*s\"",
 			scanned, start);
@@ -742,6 +749,32 @@ ParseExpr(
 		} else if (Tcl_GetBooleanFromObj(NULL,literal,&b) == TCL_OK) {
 		    lexeme = BOOLEAN;
 		} else {
+		    /*
+		     * Tricky case: see test expr-62.10
+		     */
+
+		    int scanned2 = scanned;
+		    do {
+			scanned2 += TclParseAllWhiteSpace(
+				start + scanned2, numBytes - scanned2);
+			scanned2 += ParseLexeme(
+				start + scanned2, numBytes - scanned2, &lexeme,
+				NULL);
+		    } while (lexeme == COMMENT);
+		    if (lexeme == OPEN_PAREN) {
+			/*
+			 * Actually a function call, but with obscuring
+			 * comments.  Skip to the start of the parentheses.
+			 * Note that we assume that open parentheses are one
+			 * byte long.
+			 */
+
+			lexeme = FUNCTION;
+			Tcl_ListObjAppendElement(NULL, funcList, literal);
+			scanned = scanned2 - 1;
+			break;
+		    }
+
 		    Tcl_DecrRefCount(literal);
 		    msg = Tcl_ObjPrintf("invalid bareword \"%.*s%s\"",
 			    (scanned < limit) ? scanned : limit - 3, start,
@@ -1894,7 +1927,7 @@ ParseLexeme(
 				   storage, if non-NULL. */
 {
     const char *end;
-    int scanned;
+    int scanned, size;
     Tcl_UniChar ch = 0;
     Tcl_Obj *literal = NULL;
     unsigned char byte;
@@ -1909,6 +1942,16 @@ ParseLexeme(
 	return 1;
     }
     switch (byte) {
+    case '#':
+	/*
+	 * Scan forward over the comment contents.
+	 */
+	for (size = 0; byte != '\n' && byte != 0 && size < numBytes; size++) {
+	    byte = UCHAR(start[size]);
+	}
+	*lexemePtr = COMMENT;
+	return size - (byte == '\n');
+
     case '*':
 	if ((numBytes > 1) && (start[1] == '*')) {
 	    *lexemePtr = EXPON;
diff --git a/tests/compExpr.test b/tests/compExpr.test
index 35d7588..4ef155b 100644
--- a/tests/compExpr.test
+++ b/tests/compExpr.test
@@ -371,10 +371,46 @@ test compExpr-7.2 {[Bug 1869989]: expr parser memleak} -constraints memory -setu
     unset end i tmp
     rename getbytes {}
 } -result 0
+
+proc extract {opcodes descriptor} {
+    set instructions [dict values [dict get $descriptor instructions]]
+    return [lmap i $instructions {
+	if {[lindex $i 0] in $opcodes} {string cat $i} else continue
+    }]
+}
+
+test compExpr-8.1 {TIP 582: expression comments} -setup {} -body {
+    extract {loadStk add} [tcl::unsupported::getbytecode script {expr {
+        $abc
+	# + $def
+	+ $ghi
+    }}]
+} -result {loadStk loadStk add}
+test compExpr-8.2 {TIP 582: expression comments} -setup {} -body {
+    extract {loadStk add} [tcl::unsupported::getbytecode script {expr {
+        $abc
+	# + $def
+	# + $ghi }}]
+} -result loadStk
+test compExpr-8.3 {TIP 582: expression comments} -setup {} -body {
+    extract {loadStk add} [tcl::unsupported::getbytecode script {expr {
+        $abc
+	# + $def\
+	+ $ghi
+    }}]
+} -result loadStk
+test compExpr-8.4 {TIP 582: expression comments} -setup {} -body {
+    extract {loadStk add} [tcl::unsupported::getbytecode script {expr {
+        $abc
+	# + $def\\
+	+ $ghi
+    }}]
+} -result {loadStk loadStk add}
 
 # cleanup
 catch {unset a}
 catch {unset b}
+catch {rename extract ""}
 ::tcltest::cleanupTests
 return
 
diff --git a/tests/expr-old.test b/tests/expr-old.test
index 914530e..327faa2 100644
--- a/tests/expr-old.test
+++ b/tests/expr-old.test
@@ -524,7 +524,7 @@ test expr-old-26.10b {error conditions} ieeeFloatingPoint {
     list [catch {expr 2.0/0.0} msg] $msg
 } {0 Inf}
 test expr-old-26.11 {error conditions} -body {
-    expr 2#
+    expr 2`
 } -returnCodes error -match glob -result *
 test expr-old-26.12 {error conditions} -body {
     expr a.b
diff --git a/tests/expr.test b/tests/expr.test
index 5e00841..da5a23d 100644
--- a/tests/expr.test
+++ b/tests/expr.test
@@ -7384,6 +7384,67 @@ foreach v1 $values r1 $results {
     }
 }
 unset -nocomplain values results ctr
+
+test expr-62.1 {TIP 582: comments} -body {
+    expr {1 # + 2}
+} -result 1
+test expr-62.2 {TIP 582: comments} -body {
+    expr "1 #\n+ 2"
+} -result 3
+test expr-62.3 {TIP 582: comments} -setup {
+    set ctr 0
+} -body {
+    expr {
+	# This is a demonstration of a comment
+	1 + 2 + 3
+	# and another comment
+	+ 4 + 5
+	# + [incr ctr]
+	+ [incr ctr]
+    }
+} -result 16
+# Buggy because line breaks aren't tracked inside expressions at all
+test expr-62.4 {TIP 582: comments don't hide line breaks} -setup {
+    proc getline {} {
+	dict get [info frame -1] line
+    }
+    set base [getline]
+} -constraints knownBug -body {
+    expr {
+	0
+	# a comment
+	+ [getline] - $base
+    }
+} -cleanup {
+    rename getline ""
+} -result 5
+test expr-62.5 {TIP 582: comments don't splice tokens} {
+    set a False
+    expr {$a#don't splice
+ne#don't splice
+false}
+} 1
+test expr-62.6 {TIP 582: comments don't splice tokens} {
+    expr {0x2#don't splice
+ne#don't splice
+2}
+} 1
+test expr-62.7 {TIP 582: comments can go inside function calls} {
+    expr {max(1,# comment
+	      2)}
+} 2
+test expr-62.8 {TIP 582: comments can go inside function calls} {
+    expr {max(1# comment
+	      ,2)}
+} 2
+test expr-62.9 {TIP 582: comments can go inside function calls} {
+    expr {max(# comment
+	      1,2)}
+} 2
+test expr-62.10 {TIP 582: comments can go inside function calls} {
+    expr {max# comment
+	     (1,2)}
+} 2
 
 # cleanup
 unset -nocomplain a
diff --git a/tests/parseExpr.test b/tests/parseExpr.test
index 8b5e429..735dace 100644
--- a/tests/parseExpr.test
+++ b/tests/parseExpr.test
@@ -1075,6 +1075,14 @@ test parseExpr-22.21 {Bug d2ffcca163} -constraints testexprparser -body {
     testexprparser in\u0433(0) -1
 } -returnCodes error -match glob -result {missing operand*}
 
+test parseExpr-23.1 {TIP 582: comments} -constraints testexprparser -body {
+    testexprparser "7 # * 8 " -1
+} -result {- {} 0 subexpr 7 1 text 7 0 {}}
+test parseExpr-23.2 {TIP 582: comments} -constraints testexprparser -body {
+    testexprparser "7 #\n* 8 " -1
+} -result {- {} 0 subexpr {7 #
+*} 5 operator # 0 subexpr 7 1 text 7 0 subexpr * 1 text * 0 {}}
+
 # cleanup
 cleanupTests
 return