Re: Why 'statement may have no effect' lint warning here?

[arnold]

arnold@skeeve.com wrote:

> All of the below will not produce a warning.  `a = a' is an interesting
> case, as are some of the others, but I don't feel like overly investing
> in this feature.

I am pushing the following patch.  Thanks everyone.

Arnold
---------------
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.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'