Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/usr.bin/xlint/lint1 lint: reduce number of places where 'rea...



details:   https://anonhg.NetBSD.org/src/rev/245bc8397703
branches:  trunk
changeset: 1019849:245bc8397703
user:      rillig <rillig%NetBSD.org@localhost>
date:      Sun Mar 21 18:58:34 2021 +0000

description:
lint: reduce number of places where 'reached' is set

When determining the reachability of a statement, the idea was that
whenever 'reached' was set to false, 'rchflg' (the abbreviation for "do
not warn about unreachable statements") would be reset as well.

In some (trivial) cases, this was done, but many more interesting cases
simply forgot to set this second variable.  To prevent this in the
future, encapsulate this in a simple helper function.

Now even if a statement is reachable, 'rchflg' gets reset.  This does
not hurt since as long as the current statement is reachable, the value
of 'rchflg' does not matter.

No functional change.  There would be quite a big functional change
though if check_statement_reachable were to reset 'rchflg' instead of
'reached', as the comment already suggests.  In that case, with the
current code, many legitimate warnings about unreachable statements
would be skipped, especially those involving 'if' statements, since
these didn't reset 'rchflg' properly before.

diffstat:

 usr.bin/xlint/lint1/func.c |  83 ++++++++++++++++++++++++++-------------------
 usr.bin/xlint/lint1/tree.c |   7 ++-
 2 files changed, 53 insertions(+), 37 deletions(-)

diffs (truncated from 344 to 300 lines):

diff -r 9bc00894415c -r 245bc8397703 usr.bin/xlint/lint1/func.c
--- a/usr.bin/xlint/lint1/func.c        Sun Mar 21 16:58:07 2021 +0000
+++ b/usr.bin/xlint/lint1/func.c        Sun Mar 21 18:58:34 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: func.c,v 1.92 2021/03/21 15:44:57 rillig Exp $ */
+/*     $NetBSD: func.c,v 1.93 2021/03/21 18:58:34 rillig Exp $ */
 
 /*
  * Copyright (c) 1994, 1995 Jochen Pohl
@@ -37,7 +37,7 @@
 
 #include <sys/cdefs.h>
 #if defined(__RCSID) && !defined(lint)
-__RCSID("$NetBSD: func.c,v 1.92 2021/03/21 15:44:57 rillig Exp $");
+__RCSID("$NetBSD: func.c,v 1.93 2021/03/21 18:58:34 rillig Exp $");
 #endif
 
 #include <stdlib.h>
@@ -187,6 +187,13 @@
        free(ci);
 }
 
+static void
+set_reached(bool new_reached)
+{
+       reached = new_reached;
+       rchflg = false;
+}
+
 /*
  * Prints a warning if a statement cannot be reached.
  */
@@ -196,6 +203,11 @@
        if (!reached && !rchflg) {
                /* statement not reached */
                warning(193);
+               /*
+                * FIXME: Setting 'reached = true' is wrong since the
+                * statement doesn't magically become reachable just by
+                * issuing a warning.  This must be 'rchflg = true' instead.
+                */
                reached = true; /* only to suppress further warnings */
        }
 }
@@ -334,7 +346,7 @@
        if (dcs->d_notyp)
                fsym->s_return_type_implicit_int = true;
 
-       reached = true;
+       set_reached(true);
 }
 
 static void
@@ -410,7 +422,7 @@
        rmsyms(dcs->d_func_proto_syms);
 
        /* must be set on level 0 */
-       reached = true;
+       set_reached(true);
 }
 
 void
@@ -424,7 +436,7 @@
                mark_as_set(sym);
        }
 
-       reached = true;
+       set_reached(true);
 }
 
 static void
@@ -536,7 +548,7 @@
 
        tfreeblk();
 
-       reached = true;
+       set_reached(true);
 }
 
 void
@@ -563,7 +575,7 @@
                ci->c_default = true;
        }
 
-       reached = true;
+       set_reached(true);
 }
 
 static tnode_t *
@@ -607,7 +619,9 @@
        pushctrl(T_IF);
 
        if (tn != NULL && tn->tn_op == CON && !tn->tn_system_dependent) {
-               reached = constant_is_nonzero(tn);
+               /* XXX: what if inside 'if (0)'? */
+               set_reached(constant_is_nonzero(tn));
+               /* XXX: what about always_else? */
                cstmt->c_always_then = reached;
        }
 }
@@ -621,7 +635,8 @@
 {
 
        cstmt->c_reached_end_of_then = reached;
-       reached = !cstmt->c_always_then;
+       /* XXX: what if inside 'if (0)'? */
+       set_reached(!cstmt->c_always_then);
 }
 
 /*
@@ -632,11 +647,11 @@
 if3(bool els)
 {
        if (cstmt->c_reached_end_of_then)
-               reached = true;
+               set_reached(true);
        else if (cstmt->c_always_then)
-               reached = false;
+               set_reached(false);
        else if (!els)
-               reached = true;
+               set_reached(true);
 
        popctrl(T_IF);
 }
@@ -689,7 +704,7 @@
        cstmt->c_switch = true;
        cstmt->c_swtype = tp;
 
-       reached = rchflg = false;
+       set_reached(false);
        seen_fallthrough = true;
 }
 
@@ -732,14 +747,14 @@
                 * end of switch always reached (c_break is only set if the
                 * break statement can be reached).
                 */
-               reached = true;
+               set_reached(true);
        } else if (!cstmt->c_default &&
                   (!hflag || !cstmt->c_swtype->t_is_enum || nenum != nclab)) {
                /*
                 * there are possible values which are not handled in
                 * switch
                 */
-               reached = true;
+               set_reached(true);
        }       /*
                 * otherwise the end of the switch expression is reached
                 * if the end of the last statement inside it is reached.
@@ -759,7 +774,8 @@
        if (!reached) {
                /* loop not entered at top */
                warning(207);
-               reached = true;
+               /* FIXME: that's plain wrong. */
+               set_reached(true);
        }
 
        if (tn != NULL)
@@ -773,7 +789,7 @@
        check_getopt_begin_while(tn);
        expr(tn, false, true, true, false);
 
-       reached = body_reached;
+       set_reached(body_reached);
 }
 
 /*
@@ -788,8 +804,7 @@
         * The end of the loop can be reached if it is no endless loop
         * or there was a break statement which was reached.
         */
-       reached = !cstmt->c_maybe_endless || cstmt->c_break;
-       rchflg = false;
+       set_reached(!cstmt->c_maybe_endless || cstmt->c_break);
 
        check_getopt_end_while();
        popctrl(T_WHILE);
@@ -805,7 +820,7 @@
        if (!reached) {
                /* loop not entered at top */
                warning(207);
-               reached = true;
+               set_reached(true);
        }
 
        pushctrl(T_DO);
@@ -825,7 +840,7 @@
         * loop is reached.
         */
        if (cstmt->c_continue)
-               reached = true;
+               set_reached(true);
 
        if (tn != NULL)
                tn = check_controlling_expression(tn);
@@ -840,10 +855,9 @@
        expr(tn, false, true, true, true);
 
        if (cstmt->c_maybe_endless)
-               reached = false;
+               set_reached(false);
        if (cstmt->c_break)
-               reached = true;
-       rchflg = false;
+               set_reached(true);
 
        popctrl(T_DO);
 }
@@ -862,7 +876,7 @@
        if (tn1 != NULL && !reached) {
                /* loop not entered at top */
                warning(207);
-               reached = true;
+               set_reached(true);
        }
 
        pushctrl(T_FOR);
@@ -890,7 +904,7 @@
 
        /* Checking the reinitialization expression is done in for2() */
 
-       reached = !is_zero(tn2);
+       set_reached(!is_zero(tn2));
 }
 
 /*
@@ -904,7 +918,7 @@
        tnode_t *tn3;
 
        if (cstmt->c_continue)
-               reached = true;
+               set_reached(true);
 
        cpos = curr_pos;
        cspos = csrc_pos;
@@ -919,7 +933,7 @@
        if (!reached && !rchflg) {
                /* end-of-loop code not reached */
                warning(223);
-               reached = true;
+               set_reached(true);
        }
 
        if (tn3 != NULL) {
@@ -933,8 +947,7 @@
 
        /* An endless loop without break will never terminate */
        /* TODO: What if the loop contains a 'return'? */
-       reached = cstmt->c_break || !cstmt->c_maybe_endless;
-       rchflg = false;
+       set_reached(cstmt->c_break || !cstmt->c_maybe_endless);
 
        popctrl(T_FOR);
 }
@@ -950,7 +963,7 @@
 
        check_statement_reachable();
 
-       reached = rchflg = false;
+       set_reached(false);
 }
 
 /*
@@ -976,7 +989,7 @@
        if (bflag)
                check_statement_reachable();
 
-       reached = rchflg = false;
+       set_reached(false);
 }
 
 /*
@@ -1000,7 +1013,7 @@
 
        check_statement_reachable();
 
-       reached = rchflg = false;
+       set_reached(false);
 }
 
 /*
@@ -1068,7 +1081,7 @@
 
        }
 
-       reached = rchflg = false;
+       set_reached(false);
 }
 
 /*
@@ -1257,7 +1270,7 @@
 not_reached(int n)
 {
 
-       reached = false;
+       set_reached(false);
        rchflg = true;



Home | Main Index | Thread Index | Old Index