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; } }