Fixes for lint check of no effect.

8 files changed, 106 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 17f06325..64dba79d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2021-04-14         Arnold D. Robbins     <arnold@skeeve.com>
+
+	Fix up lint warnings for "no effect" to avoid false warnings.
+	Straighten out the messages, and remove the run-time warning,
+	since a parse-time warning is already issued. Thanks to
+	Arkadiusz Drabczyk <arkadiusz@drabczyk.org> for the initial
+	bug report.
+
+	* awkgram.y (isnoeffect): Add more opcodes that are "no effect".
+	(add_lint): For LINT_no_effect, check that all the opcodes in the
+	list are "no effect". Only if so, issue the warning. Remove the
+	addition of the run-time warning.
+	* interpret.h (r_interpret): Remove LINT_no_effect from Op_lint
+	case.
+
 2021-02-13         Arnold D. Robbins     <arnold@skeeve.com>
 
 	* io.c (nextfile): Use the value of ARGC directly in the for
diff --git a/awkgram.c b/awkgram.c
index 9c2700a3..bd41a1a1 100644
--- a/awkgram.c
+++ b/awkgram.c
@@ -7845,6 +7845,16 @@ isnoeffect(OPCODE type)
 	case Op_not:
 	case Op_in_array:
 		return true;
+	// Additional opcodes that can be part of an expression
+	// that has no effect:
+	case Op_and:
+	case Op_or:
+	case Op_push:
+	case Op_push_i:
+	case Op_push_array:
+	case Op_pop:
+	case Op_lint_plus:
+		return true;
 	default:
 		break;	/* keeps gcc -Wall happy */
 	}
@@ -8658,6 +8668,7 @@ add_lint(INSTRUCTION *list, LINTTYPE linttype)
 {
 #ifndef NO_LINT
 	INSTRUCTION *ip;
+	bool no_effect = true;
 
 	switch (linttype) {
 	case LINT_assign_in_cond:
@@ -8678,26 +8689,33 @@ add_lint(INSTRUCTION *list, LINTTYPE linttype)
 		if (list->lasti->opcode == Op_pop && list->nexti != list->lasti) {
 			int line = 0;
 
-			// Get down to the last instruction (FIXME: why?)
+			// Get down to the last instruction ...
 			for (ip = list->nexti; ip->nexti != list->lasti; ip = ip->nexti) {
-				// along the way track line numbers, we will use the line
+				// ... along the way track line numbers, we will use the line
 				// closest to the opcode if that opcode doesn't have one
 				if (ip->source_line != 0)
 					line = ip->source_line;
+
+				// And check each opcode for no effect
+				no_effect = no_effect && isnoeffect(ip->opcode);
 			}
 
-			if (do_lint) {		/* compile-time warning */
-				if (isnoeffect(ip->opcode)) {
+			// check the last one also
+			no_effect = no_effect && isnoeffect(ip->opcode);
+
+			// Only if all the traversed opcodes have no effect do we
+			// produce a warning. This avoids warnings for things like
+			// a == b && b = c.
+			if (do_lint) {		/* parse-time warning */
+				if (no_effect) {
 					if (ip->source_line != 0)
 						line = ip->source_line;
-					lintwarn_ln(line, ("statement may have no effect"));
+					lintwarn_ln(line, _("statement has no effect"));
 				}
 			}
 
-			if (ip->opcode == Op_push || ip->opcode == Op_push_i) {		/* run-time warning */
-				list_append(list, instruction(Op_lint));
-				list->lasti->lint_type = linttype;
-			}
+			// We no longer place a run-time warning also. One warning
+			// at parse time is enough.
 		}
 		break;
 
diff --git a/awkgram.y b/awkgram.y
index 6dfbe7e0..e4c9708d 100644
--- a/awkgram.y
+++ b/awkgram.y
@@ -5343,6 +5343,16 @@ isnoeffect(OPCODE type)
 	case Op_not:
 	case Op_in_array:
 		return true;
+	// Additional opcodes that can be part of an expression
+	// that has no effect:
+	case Op_and:
+	case Op_or:
+	case Op_push:
+	case Op_push_i:
+	case Op_push_array:
+	case Op_pop:
+	case Op_lint_plus:
+		return true;
 	default:
 		break;	/* keeps gcc -Wall happy */
 	}
@@ -6156,6 +6166,7 @@ add_lint(INSTRUCTION *list, LINTTYPE linttype)
 {
 #ifndef NO_LINT
 	INSTRUCTION *ip;
+	bool no_effect = true;
 
 	switch (linttype) {
 	case LINT_assign_in_cond:
@@ -6176,26 +6187,33 @@ add_lint(INSTRUCTION *list, LINTTYPE linttype)
 		if (list->lasti->opcode == Op_pop && list->nexti != list->lasti) {
 			int line = 0;
 
-			// Get down to the last instruction (FIXME: why?)
+			// Get down to the last instruction ...
 			for (ip = list->nexti; ip->nexti != list->lasti; ip = ip->nexti) {
-				// along the way track line numbers, we will use the line
+				// ... along the way track line numbers, we will use the line
 				// closest to the opcode if that opcode doesn't have one
 				if (ip->source_line != 0)
 					line = ip->source_line;
+
+				// And check each opcode for no effect
+				no_effect = no_effect && isnoeffect(ip->opcode);
 			}
 
-			if (do_lint) {		/* compile-time warning */
-				if (isnoeffect(ip->opcode)) {
+			// check the last one also
+			no_effect = no_effect && isnoeffect(ip->opcode);
+
+			// Only if all the traversed opcodes have no effect do we
+			// produce a warning. This avoids warnings for things like
+			// a == b && b = c.
+			if (do_lint) {		/* parse-time warning */
+				if (no_effect) {
 					if (ip->source_line != 0)
 						line = ip->source_line;
-					lintwarn_ln(line, ("statement may have no effect"));
+					lintwarn_ln(line, _("statement has no effect"));
 				}
 			}
 
-			if (ip->opcode == Op_push || ip->opcode == Op_push_i) {		/* run-time warning */
-				list_append(list, instruction(Op_lint));
-				list->lasti->lint_type = linttype;
-			}
+			// We no longer place a run-time warning also. One warning
+			// at parse time is enough.
 		}
 		break;
 
diff --git a/interpret.h b/interpret.h
index 739f81eb..2ed4f01a 100644
--- a/interpret.h
+++ b/interpret.h
@@ -414,10 +414,6 @@ uninitialized_scalar:
 					lintwarn(_("assignment used in conditional context"));
 					break;
 
-				case LINT_no_effect:
-					lintwarn(_("statement has no effect"));
-					break;
-
 				default:
 					cant_happen();
 				}
diff --git a/test/ChangeLog b/test/ChangeLog
index 53fe5627..641a9849 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,9 @@
+2021-04-14         Arnold D. Robbins     <arnold@skeeve.com>
+
+	* noeffect.awk: Add more test cases. Thanks to Wolfgang Laun
+	<wolfgang.laun@gmail.com>.
+	* lintwarn.ok, noeffect.ok: Updated after code and test changes.
+
 2021-02-13         Arnold D. Robbins     <arnold@skeeve.com>
 
 	* Makefile.am (EXTRA_DIST): argcasfile, new test.
diff --git a/test/lintwarn.ok b/test/lintwarn.ok
index d0993ea7..4f503b46 100644
--- a/test/lintwarn.ok
+++ b/test/lintwarn.ok
@@ -1,15 +1,15 @@
 gawk: lintwarn.awk:2: warning: `BEGINFILE' is a gawk extension
 gawk: lintwarn.awk:3: error: non-redirected `getline' invalid inside `BEGINFILE' rule
 gawk: lintwarn.awk:4: error: non-redirected `getline' invalid inside `BEGINFILE' rule
-gawk: lintwarn.awk:8: warning: statement may have no effect
+gawk: lintwarn.awk:8: warning: statement has no effect
 gawk: lintwarn.awk:9: warning: plain `print' in BEGIN or END rule should probably be `print ""'
 gawk: lintwarn.awk:10: error: `nextfile' used in BEGIN action
 gawk: lintwarn.awk:12: warning: `delete(array)' is a non-portable tawk extension
 gawk: lintwarn.awk:13: warning: regular expression on right of assignment
 gawk: lintwarn.awk:14: warning: regular expression on right of comparison
-gawk: lintwarn.awk:14: warning: statement may have no effect
+gawk: lintwarn.awk:14: warning: statement has no effect
 gawk: lintwarn.awk:15: warning: regular expression on left of `~' or `!~' operator
-gawk: lintwarn.awk:15: warning: statement may have no effect
+gawk: lintwarn.awk:15: warning: statement has no effect
 gawk: lintwarn.awk:16: warning: call of `length' without parentheses is not portable
 gawk: lintwarn.awk:17: warning: `switch' is a gawk extension
 gawk: lintwarn.awk:18: warning: `case' is a gawk extension
@@ -23,12 +23,12 @@ gawk: lintwarn.awk:25: error: `next' used in BEGIN action
 gawk: lintwarn.awk:26: 	a[]
 gawk: lintwarn.awk:26: 	  ^ syntax error
 gawk: lintwarn.awk:26: error: invalid subscript expression
-gawk: lintwarn.awk:26: warning: statement may have no effect
+gawk: lintwarn.awk:26: warning: statement has no effect
 gawk: lintwarn.awk:27: warning: regexp constant for parameter #1 yields boolean value
 gawk: lintwarn.awk:28: warning: regexp constant `//' looks like a C++ comment, but is not
-gawk: lintwarn.awk:28: warning: statement may have no effect
+gawk: lintwarn.awk:28: warning: statement has no effect
 gawk: lintwarn.awk:29: warning: regexp constant `/* */' looks like a C comment, but is not
-gawk: lintwarn.awk:29: warning: statement may have no effect
+gawk: lintwarn.awk:29: warning: statement has no effect
 gawk: lintwarn.awk:32: warning: non-redirected `getline' undefined inside END action
 gawk: lintwarn.awk:34: error: function `zz': parameter #2, `aa', duplicates parameter #1
 gawk: lintwarn.awk:38: warning: `include' is a gawk extension
diff --git a/test/noeffect.awk b/test/noeffect.awk
index 4be76b1e..107ccbe5 100644
--- a/test/noeffect.awk
+++ b/test/noeffect.awk
@@ -7,4 +7,22 @@ BEGIN {
 	42
 	q = 42
 	q
+
+	a = b = 42
+
+	a * b
+	a != b
+	# the following should not produce warnings
+	a++ == a--
+	f_without_side_effect(a);
+	f_with_side_effect(b) == 2
+	1 == 2 && a++
+	1 == 1 || b--
+	a = a
+	a *= 1
+	a += 0
+	a*a < 0 && b = 1001
 }
+
+function f_without_side_effect(x) { }
+function f_with_side_effect(x) { }
diff --git a/test/noeffect.ok b/test/noeffect.ok
index 9778a4bb..38dc9dd4 100644
--- a/test/noeffect.ok
+++ b/test/noeffect.ok
@@ -1,8 +1,10 @@
-gawk: noeffect.awk:2: warning: statement may have no effect
-gawk: noeffect.awk:3: warning: statement may have no effect
-gawk: noeffect.awk:5: warning: statement may have no effect
-gawk: noeffect.awk:2: warning: reference to uninitialized variable `s'
-gawk: noeffect.awk:3: warning: reference to uninitialized variable `s'
+gawk: noeffect.awk:2: warning: statement has no effect
+gawk: noeffect.awk:3: warning: statement has no effect
+gawk: noeffect.awk:5: warning: statement has no effect
 gawk: noeffect.awk:6: warning: statement has no effect
 gawk: noeffect.awk:7: warning: statement has no effect
 gawk: noeffect.awk:9: warning: statement has no effect
+gawk: noeffect.awk:13: warning: statement has no effect
+gawk: noeffect.awk:14: warning: statement has no effect
+gawk: noeffect.awk:2: warning: reference to uninitialized variable `s'
+gawk: noeffect.awk:3: warning: reference to uninitialized variable `s'