diff options
| -rw-r--r-- | ChangeLog | 15 | ||||
| -rw-r--r-- | awkgram.c | 36 | ||||
| -rw-r--r-- | awkgram.y | 36 | ||||
| -rw-r--r-- | interpret.h | 4 | ||||
| -rw-r--r-- | test/ChangeLog | 6 | ||||
| -rw-r--r-- | test/lintwarn.ok | 12 | ||||
| -rw-r--r-- | test/noeffect.awk | 18 | ||||
| -rw-r--r-- | test/noeffect.ok | 12 |
8 files changed, 106 insertions, 33 deletions
@@ -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 @@ -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; @@ -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' |