Module Name: src
Committed By: rillig
Date: Sat Jan 30 23:05:08 UTC 2021
Modified Files:
src/tests/usr.bin/xlint/lint1: msg_129.c msg_129.exp
src/usr.bin/xlint/lint1: tree.c
Log Message:
lint: fix wrong 'expression has null effect'
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/tests/usr.bin/xlint/lint1/msg_129.c \
src/tests/usr.bin/xlint/lint1/msg_129.exp
cvs rdiff -u -r1.197 -r1.198 src/usr.bin/xlint/lint1/tree.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/tests/usr.bin/xlint/lint1/msg_129.c
diff -u src/tests/usr.bin/xlint/lint1/msg_129.c:1.2 src/tests/usr.bin/xlint/lint1/msg_129.c:1.3
--- src/tests/usr.bin/xlint/lint1/msg_129.c:1.2 Sat Jan 30 22:38:54 2021
+++ src/tests/usr.bin/xlint/lint1/msg_129.c Sat Jan 30 23:05:08 2021
@@ -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 @@ uint8_buffer_write_uint32(uint8_t *c, ui
(*(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;
}
Index: src/tests/usr.bin/xlint/lint1/msg_129.exp
diff -u src/tests/usr.bin/xlint/lint1/msg_129.exp:1.2 src/tests/usr.bin/xlint/lint1/msg_129.exp:1.3
--- src/tests/usr.bin/xlint/lint1/msg_129.exp:1.2 Sat Jan 30 22:38:54 2021
+++ src/tests/usr.bin/xlint/lint1/msg_129.exp Sat Jan 30 23:05:08 2021
@@ -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]
Index: src/usr.bin/xlint/lint1/tree.c
diff -u src/usr.bin/xlint/lint1/tree.c:1.197 src/usr.bin/xlint/lint1/tree.c:1.198
--- src/usr.bin/xlint/lint1/tree.c:1.197 Sat Jan 30 22:48:50 2021
+++ src/usr.bin/xlint/lint1/tree.c Sat Jan 30 23:05:08 2021
@@ -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 @@ has_side_effect(const tnode_t *tn)
*/
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;
}
}
