[src/trunk]: src/usr.bin/xlint/lint1 lint: fix wrong 'expression has null eff...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/825c0fd66733
branches:  trunk
changeset: 1018328:825c0fd66733
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sat Jan 30 23:05:08 2021 +0000

description:
lint: fix wrong 'expression has null effect'

diffstat:

 tests/usr.bin/xlint/lint1/msg_129.c   |  31 ++++++++++++++++++++++++-------
 tests/usr.bin/xlint/lint1/msg_129.exp |   3 ++-
 usr.bin/xlint/lint1/tree.c            |  19 +++++--------------
 3 files changed, 31 insertions(+), 22 deletions(-)

diffs (100 lines):

diff -r 1ac9951dd103 -r 825c0fd66733 tests/usr.bin/xlint/lint1/msg_129.c
--- a/tests/usr.bin/xlint/lint1/msg_129.c       Sat Jan 30 22:48:50 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/msg_129.c       Sat Jan 30 23:05:08 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: msg_129.c,v 1.2 2021/01/30 22:38:54 rillig Exp $       */
+/*     $NetBSD: msg_129.c,v 1.3 2021/01/30 23:05:08 rillig Exp $       */
 # 3 "msg_129.c"
 
 // Test for message: expression has null effect [129]
@@ -8,13 +8,18 @@
 typedef unsigned char uint8_t;
 typedef unsigned int uint32_t;
 
+_Bool side_effect(void);
+
 /*
- * XXX: The message 129 looks wrong in this case.  There are several comma
- * operators, each of them has an assignment as operand, and an assignment
- * has side effects.
+ * Before tree.c 1.198 from 2021-01-30, the nested comma operators were
+ * wrongly reported as having no side effect.
  *
- * Nevertheless, when stepping through check_null_effect, the operator ','
- * in line 17 says it has no side effect, which is strange.
+ * The bug was that has_side_effect did not properly examine the sub-nodes.
+ * The ',' operator has m_has_side_effect == false since it depends on its
+ * operands whether the ',' actually has side effects.  For nested ','
+ * operators, the function did not evaluate the operands deeply but only did
+ * a quick shallow test on the m_has_side_effect property.  Since that is
+ * false, lint thought that the whole expression would have no side effect.
  */
 void
 uint8_buffer_write_uint32(uint8_t *c, uint32_t l)
@@ -22,5 +27,17 @@
        (*(c++) = (uint8_t)(l & 0xff),
            *(c++) = (uint8_t)((l >> 8L) & 0xff),
            *(c++) = (uint8_t)((l >> 16L) & 0xff),
-           *(c++) = (uint8_t)((l >> 24L) & 0xff));     /* expect: 129 */
+           *(c++) = (uint8_t)((l >> 24L) & 0xff));
 }
+
+void
+operator_comma(void)
+{
+       side_effect(), 0;               /* the 0 is redundant */
+       0, side_effect();               /* expect: 129 */
+
+       if (side_effect(), 0)           /* the 0 controls the 'if' */
+               return;
+       if (0, side_effect())           /* expect: 129 */
+               return;
+}
diff -r 1ac9951dd103 -r 825c0fd66733 tests/usr.bin/xlint/lint1/msg_129.exp
--- a/tests/usr.bin/xlint/lint1/msg_129.exp     Sat Jan 30 22:48:50 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/msg_129.exp     Sat Jan 30 23:05:08 2021 +0000
@@ -1,1 +1,2 @@
-msg_129.c(25): warning: expression has null effect [129]
+msg_129.c(37): warning: expression has null effect [129]
+msg_129.c(41): warning: expression has null effect [129]
diff -r 1ac9951dd103 -r 825c0fd66733 usr.bin/xlint/lint1/tree.c
--- a/usr.bin/xlint/lint1/tree.c        Sat Jan 30 22:48:50 2021 +0000
+++ b/usr.bin/xlint/lint1/tree.c        Sat Jan 30 23:05:08 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tree.c,v 1.197 2021/01/30 22:48:50 rillig Exp $        */
+/*     $NetBSD: tree.c,v 1.198 2021/01/30 23:05:08 rillig Exp $        */
 
 /*
  * Copyright (c) 1994, 1995 Jochen Pohl
@@ -37,7 +37,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: tree.c,v 1.197 2021/01/30 22:48:50 rillig Exp $");
+__RCSID("$NetBSD: tree.c,v 1.198 2021/01/30 23:05:08 rillig Exp $");
 #endif
 
 #include <float.h>
@@ -3789,19 +3789,10 @@
                         */
                        tn = tn->tn_right;
                } else if (tn->tn_op == COLON || tn->tn_op == COMMA) {
-                       /*
-                        * : has a side effect if at least one of its operands
-                        * has a side effect
-                        */
-                       if (modtab[tn->tn_left->tn_op].m_has_side_effect) {
-                               tn = tn->tn_left;
-                       } else if (modtab[tn->tn_right->tn_op].m_has_side_effect) {
-                               tn = tn->tn_right;
-                       } else {
-                               break;
-                       }
+                       return has_side_effect(tn->tn_left) ||
+                              has_side_effect(tn->tn_right);
                } else {
-                       break;
+                       return false;
                }
        }