[src/trunk]: src/usr.bin/xlint/lint1 tests/lint: explain the missing error me...

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

details:   https://anonhg.NetBSD.org/src/rev/adebf695964d
branches:  trunk
changeset: 1026279:adebf695964d
user:      rillig <rillig%NetBSD.org@localhost>
date:      Tue Nov 16 06:55:03 2021 +0000

description:
tests/lint: explain the missing error message in strict bool mode

There are 2 separate bugs that lead to the missing error message.

The first bug is that when parsing a name, the information about whether
that name comes from a system header is not properly recorded for the
last token before switching from or to a system header.

The second bug is that for determining whether the type of a function
call expression is relaxed, the types of the function call arguments are
taken into account, even though the type only depends on the function
itself.  The same idea applies to the operators '<<' and '>>', as their
result type only depends on the left operand, at least since C90.

diffstat:

 tests/usr.bin/xlint/lint1/d_c99_bool_strict.c   |  62 +++++++++++++++++++++---
 tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp |  11 ++--
 usr.bin/xlint/lint1/debug.c                     |   9 ++-
 usr.bin/xlint/lint1/mem1.c                      |   9 ++-
 usr.bin/xlint/lint1/tree.c                      |   7 ++-
 5 files changed, 76 insertions(+), 22 deletions(-)

diffs (211 lines):

diff -r 00804e382546 -r adebf695964d tests/usr.bin/xlint/lint1/d_c99_bool_strict.c
--- a/tests/usr.bin/xlint/lint1/d_c99_bool_strict.c     Tue Nov 16 06:46:41 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/d_c99_bool_strict.c     Tue Nov 16 06:55:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: d_c99_bool_strict.c,v 1.31 2021/11/14 11:23:52 rillig Exp $    */
+/*     $NetBSD: d_c99_bool_strict.c,v 1.32 2021/11/16 06:55:03 rillig Exp $    */
 # 3 "d_c99_bool_strict.c"
 
 /*
@@ -777,6 +777,15 @@
        };
 }
 
+/*
+ * For expressions that originate from a system header, the strict type rules
+ * are relaxed a bit, to allow for expressions like 'flags & FLAG', even
+ * though they are not strictly boolean.
+ *
+ * This shouldn't apply to function call expressions though since one of the
+ * goals of strict bool mode is to normalize all expressions like 'strcmp' to
+ * be 'strcmp(a, b) == 0' instead of '!strcmp(a, b)'.
+ */
 # 1 "stdio.h" 1 3 4
 typedef struct stdio_file {
        int fd;
@@ -784,10 +793,10 @@
 int ferror(FILE *);
 FILE stdio_files[3];
 FILE *stdio_stdout;
-# 788 "d_c99_bool_strict.c" 2
+# 797 "d_c99_bool_strict.c" 2
 # 1 "string.h" 1 3 4
 int strcmp(const char *, const char *);
-# 791 "d_c99_bool_strict.c" 2
+# 800 "d_c99_bool_strict.c" 2
 
 void
 controlling_expression(FILE *f, const char *a, const char *b)
@@ -816,23 +825,58 @@
         *
         * Seen in bin/echo/echo.c, function main, call to ferror.
         */
+       /*
+        * TODO: In a function call expression, tn->tn_relaxed should only be
+        * derived from the function itself, not from its arguments.
+        */
        /* TODO: Warn about type mismatch [333]. */
        if (ferror(
-# 822 "d_c99_bool_strict.c" 3 4
+# 835 "d_c99_bool_strict.c" 3 4
            &stdio_files[1]
-# 824 "d_c99_bool_strict.c"
+# 837 "d_c99_bool_strict.c"
            ))
                return;
 
        /*
-        * TODO: Why is there a difference between array access and a plain
-        * variable? Either both should get a warning or none of them.
+        * FIXME: At the end of parsing the name 'stdio_stdout', the parser
+        * has already looked ahead to the next token, to see whether it is
+        * a '(' of a function call.  At that point, the parser is no longer
+        * in a system header, therefore 'stdio_stdout' is not tn_relaxed,
+        * and this information is pushed down to the whole function call
+        * expression.
         */
        /* expect+5: error: controlling expression must be bool, not 'int' [333] */
        if (ferror(
-# 834 "d_c99_bool_strict.c" 3 4
+# 851 "d_c99_bool_strict.c" 3 4
            stdio_stdout
-# 836 "d_c99_bool_strict.c"
+# 853 "d_c99_bool_strict.c"
+           ))
+               return;
+
+       /*
+        * In this variant, there is a token ')' after the name
+        * 'stdio_stdout', which has the effect that at the end of parsing
+        * the name, the parser is still in the system header, thus setting
+        * tn_relaxed to true.
+        */
+       if (ferror(
+# 864 "d_c99_bool_strict.c" 3 4
+           (stdio_stdout)
+# 866 "d_c99_bool_strict.c"
+           ))
+               return;
+
+       /*
+        * A comment following 'stdio_stdout' does not prevent the search for
+        * '('.  At the point where build_name calls expr_zalloc_tnode, the
+        * parser is already in the main file again, thus treating
+        * stdio_stdout as not coming from a system header.
+        */
+       /* expect+5: error: controlling expression must be bool, not 'int' [333] */
+       if (ferror(
+# 878 "d_c99_bool_strict.c" 3 4
+           stdio_stdout /* comment */
+# 880 "d_c99_bool_strict.c"
            ))
                return;
 }
diff -r 00804e382546 -r adebf695964d tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp
--- a/tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp   Tue Nov 16 06:46:41 2021 +0000
+++ b/tests/usr.bin/xlint/lint1/d_c99_bool_strict.exp   Tue Nov 16 06:55:03 2021 +0000
@@ -167,8 +167,9 @@
 d_c99_bool_strict.c(764): warning: constant in conditional context [161]
 d_c99_bool_strict.c(775): error: operands of 'init' have incompatible types (_Bool != int) [107]
 d_c99_bool_strict.c(776): error: operands of 'init' have incompatible types (_Bool != int) [107]
-d_c99_bool_strict.c(796): error: controlling expression must be bool, not 'int' [333]
-d_c99_bool_strict.c(799): error: controlling expression must be bool, not 'int' [333]
-d_c99_bool_strict.c(802): error: operand of '!' must be bool, not 'int' [330]
-d_c99_bool_strict.c(805): error: operand of '!' must be bool, not 'int' [330]
-d_c99_bool_strict.c(836): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(805): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(808): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(811): error: operand of '!' must be bool, not 'int' [330]
+d_c99_bool_strict.c(814): error: operand of '!' must be bool, not 'int' [330]
+d_c99_bool_strict.c(853): error: controlling expression must be bool, not 'int' [333]
+d_c99_bool_strict.c(880): error: controlling expression must be bool, not 'int' [333]
diff -r 00804e382546 -r adebf695964d usr.bin/xlint/lint1/debug.c
--- a/usr.bin/xlint/lint1/debug.c       Tue Nov 16 06:46:41 2021 +0000
+++ b/usr.bin/xlint/lint1/debug.c       Tue Nov 16 06:55:03 2021 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: debug.c,v 1.3 2021/08/22 21:27:15 rillig Exp $ */
+/* $NetBSD: debug.c,v 1.4 2021/11/16 06:55:03 rillig Exp $ */
 
 /*-
  * Copyright (c) 2021 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: debug.c,v 1.3 2021/08/22 21:27:15 rillig Exp $");
+__RCSID("$NetBSD: debug.c,v 1.4 2021/11/16 06:55:03 rillig Exp $");
 #endif
 
 #include <stdlib.h>
@@ -117,10 +117,11 @@
 
        op = tn->tn_op;
        debug_indent();
-       debug_printf("'%s' with type '%s'%s%s",
+       debug_printf("'%s' with type '%s'%s%s%s",
            op == CVT && !tn->tn_cast ? "convert" : modtab[op].m_name,
            type_name(tn->tn_type), tn->tn_lvalue ? ", lvalue" : "",
-           tn->tn_parenthesized ? ", parenthesized" : "");
+           tn->tn_parenthesized ? ", parenthesized" : "",
+           tn->tn_relaxed ? ", relaxed" : "");
 
        if (op == NAME)
                debug_printf(" %s %s\n", tn->tn_sym->s_name,
diff -r 00804e382546 -r adebf695964d usr.bin/xlint/lint1/mem1.c
--- a/usr.bin/xlint/lint1/mem1.c        Tue Nov 16 06:46:41 2021 +0000
+++ b/usr.bin/xlint/lint1/mem1.c        Tue Nov 16 06:55:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mem1.c,v 1.52 2021/08/31 17:22:25 rillig Exp $ */
+/*     $NetBSD: mem1.c,v 1.53 2021/11/16 06:55:03 rillig Exp $ */
 
 /*
  * Copyright (c) 1994, 1995 Jochen Pohl
@@ -37,7 +37,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: mem1.c,v 1.52 2021/08/31 17:22:25 rillig Exp $");
+__RCSID("$NetBSD: mem1.c,v 1.53 2021/11/16 06:55:03 rillig Exp $");
 #endif
 
 #include <sys/param.h>
@@ -364,6 +364,11 @@
         * typically contain generated code that cannot be influenced, such
         * as a flex lexer or a yacc parser.
         */
+       /*
+        * FIXME: At this point, it is too late to check in_system_header.
+        * That must already be done at the beginning of the token.
+        * See the test d_c99_bool_strict.c, 'stdio_stdout'.
+        */
        tn->tn_relaxed = in_system_header ||
                         (curr_pos.p_file != csrc_pos.p_file &&
                          str_endswith(curr_pos.p_file, ".c"));
diff -r 00804e382546 -r adebf695964d usr.bin/xlint/lint1/tree.c
--- a/usr.bin/xlint/lint1/tree.c        Tue Nov 16 06:46:41 2021 +0000
+++ b/usr.bin/xlint/lint1/tree.c        Tue Nov 16 06:55:03 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: tree.c,v 1.393 2021/11/01 19:48:51 rillig Exp $        */
+/*     $NetBSD: tree.c,v 1.394 2021/11/16 06:55:03 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.393 2021/11/01 19:48:51 rillig Exp $");
+__RCSID("$NetBSD: tree.c,v 1.394 2021/11/16 06:55:03 rillig Exp $");
 #endif
 
 #include <float.h>
@@ -1844,6 +1844,9 @@
 
        ntn->tn_op = op;
        ntn->tn_type = type;
+       /* FIXME: For function call expressions (CALL/ICALL), ignore rn. */
+       /* FIXME: For bit shift expressions, ignore rn. */
+       /* TODO: Check all other operators for the exact combination rules. */
        ntn->tn_relaxed = ln->tn_relaxed || (rn != NULL && rn->tn_relaxed);
        ntn->tn_left = ln;
        ntn->tn_right = rn;