This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Some low-hanging -Wunreachable-code fruits.
authorJarkko Hietaniemi <jhi@iki.fi>
Fri, 13 Jun 2014 01:23:14 +0000 (21:23 -0400)
committerJarkko Hietaniemi <jhi@iki.fi>
Sat, 14 Jun 2014 01:41:58 +0000 (21:41 -0400)
- after croak/die/exit (or return), break (or return!) are pointless
  (break is not a terminator/separator, it's a promise of a jump)
- after goto, another goto (!) is pointless
- in some cases (usually function ends) introduce explicit NOT_REACHED
  to make the noreturn nature clearer (do not do this everywhere, though,
  since that would mean adding NOT_REACHED after every croak)
- for the added NOT_REACHED also add /* NOTREACHED */ since
  NOT_REACHED is for gcc (and VC), while the comment is for linters
- declaring variables in switch blocks is just too fragile:
  it kind of works for narrowing the scope (which is nice),
  but breaks the moment there are initializations for the variables
  (they will be skipped!); in some easy cases simply hoist the declarations
  out of the block and move them earlier

There are still a few places left.

16 files changed:
dump.c
ext/ExtUtils-Miniperl/lib/ExtUtils/Miniperl.pm
gv.c
mg.c
miniperlmain.c
op.c
pp.c
pp_ctl.c
pp_pack.c
pp_sys.c
regcomp.c
regexec.c
regexp.h
sv.c
toke.c
utf8.c

diff --git a/dump.c b/dump.c
index 9bbbe2d..888866c 100644 (file)
--- a/dump.c
+++ b/dump.c
@@ -2379,6 +2379,7 @@ I32
 Perl_debop(pTHX_ const OP *o)
 {
     dVAR;
+    int count;
 
     PERL_ARGS_ASSERT_DEBOP;
 
@@ -2410,9 +2411,6 @@ Perl_debop(pTHX_ const OP *o)
            PerlIO_printf(Perl_debug_log, "(NULL)");
        break;
 
-    {
-        int count;
-
     case OP_PADSV:
     case OP_PADAV:
     case OP_PADHV:
@@ -2446,7 +2444,6 @@ Perl_debop(pTHX_ const OP *o)
             PerlIO_printf(Perl_debug_log, ")");
         }
         break;
-    }
 
     default:
        break;
index cede318..94927af 100644 (file)
@@ -191,7 +191,6 @@ main(int argc, char **argv, char **env)
 #endif /* PERL_GLOBAL_STRUCT */
 
     exit(exitstatus);
-    return exitstatus;
 }
 
 /* Register any extra external extensions */
diff --git a/gv.c b/gv.c
index b4ad523..1ef1155 100644 (file)
--- a/gv.c
+++ b/gv.c
@@ -352,7 +352,7 @@ Perl_gv_init_pvn(pTHX_ GV *gv, HV *stash, const char *name, STRLEN len, U32 flag
        case SVt_PVIO:
             Perl_croak(aTHX_ "Cannot convert a reference to %s to typeglob",
                       sv_reftype(has_constant, 0));
-            break;
+
        default: NOOP;
        }
        SvRV_set(gv, NULL);
@@ -2969,7 +2969,7 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
         case regexp_amg:
             /* FAIL safe */
             return NULL;       /* Delegate operation to standard mechanisms. */
-            break;
+
         case to_sv_amg:
         case to_av_amg:
         case to_hv_amg:
@@ -2977,7 +2977,7 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
         case to_cv_amg:
             /* FAIL safe */
             return left;       /* Delegate operation to standard mechanisms. */
-            break;
+
         default:
           goto not_found;
         }
@@ -3044,7 +3044,6 @@ Perl_amagic_call(pTHX_ SV *left, SV *right, int method, int flags)
         case to_cv_amg:
             /* FAIL safe */
             return left;       /* Delegate operation to standard mechanisms. */
-            break;
       }
       if (ocvp && (cv=ocvp[nomethod_amg])) { /* Call report method */
        notfound = 1; lr = -1;
diff --git a/mg.c b/mg.c
index 63d0d61..bd09ff6 100644 (file)
--- a/mg.c
+++ b/mg.c
@@ -348,9 +348,9 @@ Perl_mg_size(pTHX_ SV *sv)
            /* FIXME */
        default:
            Perl_croak(aTHX_ "Size magic not implemented");
-           break;
+
     }
-    return 0;
+    NOT_REACHED; /* NOTREACHED */
 }
 
 /*
index f22dcbb..e748523 100644 (file)
@@ -162,7 +162,6 @@ main(int argc, char **argv, char **env)
 #endif /* PERL_GLOBAL_STRUCT */
 
     exit(exitstatus);
-    return exitstatus;
 }
 
 /* Register any extra external extensions */
diff --git a/op.c b/op.c
index eb64b52..eea56f8 100644 (file)
--- a/op.c
+++ b/op.c
@@ -1071,7 +1071,6 @@ Perl_op_contextualize(pTHX_ OP *o, I32 context)
        default:
            Perl_croak(aTHX_ "panic: op_contextualize bad context %ld",
                       (long) context);
-           return o;
     }
 }
 
@@ -9818,7 +9817,7 @@ Perl_rv2cv_op_cv(pTHX_ OP *cvop, U32 flags)
        } break;
        default: {
            return NULL;
-       } break;
+       } NOT_REACHED; /* NOTREACHED */
     }
     if (SvTYPE((SV*)cv) != SVt_PVCV)
        return NULL;
@@ -9995,7 +9994,7 @@ Perl_ck_entersub_args_proto(pTHX_ OP *entersubop, GV *namegv, SV *protosv)
                break;
            case '[': case ']':
                goto oops;
-               break;
+
            case '\\':
                proto++;
                arg++;
@@ -10010,7 +10009,7 @@ Perl_ck_entersub_args_proto(pTHX_ OP *entersubop, GV *namegv, SV *protosv)
                        else
                            goto oops;
                        goto again;
-                       break;
+
                    case ']':
                        if (contextclass) {
                            const char *p = proto;
@@ -10704,6 +10703,8 @@ Perl_rpeep(pTHX_ OP *o)
     OP** defer_queue[MAX_DEFERRED]; /* small queue of deferred branches */
     int defer_base = 0;
     int defer_ix = -1;
+    OP *fop;
+    OP *sop;
 
     if (!o || o->op_opt)
        return;
@@ -11277,10 +11278,6 @@ Perl_rpeep(pTHX_ OP *o)
 
            break;
         
-        {
-            OP *fop;
-            OP *sop;
-            
 #define HV_OR_SCALARHV(op)                                   \
     (  (op)->op_type == OP_PADHV || (op)->op_type == OP_RV2HV \
        ? (op)                                                  \
@@ -11366,8 +11363,7 @@ Perl_rpeep(pTHX_ OP *o)
            if ((fop = HV_OR_SCALARHV(cLOGOP->op_first)))
                fop->op_private |= OPpTRUEBOOL;
 #undef HV_OR_SCALARHV
-           /* GERONIMO! */
-       }    
+           /* GERONIMO! */ /* FALLTHROUGH */
 
        case OP_MAPWHILE:
        case OP_GREPWHILE:
diff --git a/pp.c b/pp.c
index 9d19c91..f544c39 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -226,8 +226,9 @@ S_rv2gv(pTHX_ SV *sv, const bool vivify_sv, const bool strict,
            SvREFCNT_inc_void_NN(sv);
            sv = MUTABLE_SV(gv);
        }
-       else if (!isGV_with_GP(sv))
-           return (SV *)Perl_die(aTHX_ "Not a GLOB reference");
+       else if (!isGV_with_GP(sv)) {
+           Perl_die(aTHX_ "Not a GLOB reference");
+        }
     }
     else {
        if (!isGV_with_GP(sv)) {
@@ -257,8 +258,9 @@ S_rv2gv(pTHX_ SV *sv, const bool vivify_sv, const bool strict,
                    SvSETMAGIC(sv);
                    goto wasref;
                }
-               if (PL_op->op_flags & OPf_REF || strict)
-                   return (SV *)Perl_die(aTHX_ PL_no_usym, "a symbol");
+               if (PL_op->op_flags & OPf_REF || strict) {
+                   Perl_die(aTHX_ PL_no_usym, "a symbol");
+                }
                if (ckWARN(WARN_UNINITIALIZED))
                    report_uninit(sv);
                return &PL_sv_undef;
@@ -271,14 +273,14 @@ S_rv2gv(pTHX_ SV *sv, const bool vivify_sv, const bool strict,
                    return &PL_sv_undef;
            }
            else {
-               if (strict)
-                   return
-                    (SV *)Perl_die(aTHX_
-                           S_no_symref_sv,
-                           sv,
-                           (SvPOKp(sv) && SvCUR(sv)>32 ? "..." : ""),
-                           "a symbol"
-                          );
+               if (strict) {
+                    Perl_die(aTHX_
+                             S_no_symref_sv,
+                             sv,
+                             (SvPOKp(sv) && SvCUR(sv)>32 ? "..." : ""),
+                             "a symbol"
+                             );
+                }
                if ((PL_op->op_private & (OPpLVAL_INTRO|OPpDONT_INIT_GV))
                    == OPpDONT_INIT_GV) {
                    /* We are the target of a coderef assignment.  Return
index ab729b0..8957a8c 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1357,9 +1357,8 @@ Perl_block_gimme(pTHX)
        return G_ARRAY;
     default:
        Perl_croak(aTHX_ "panic: bad gimme: %d\n", cxstack[cxix].blk_gimme);
-       assert(0); /* NOTREACHED */
-       return 0;
     }
+    NOT_REACHED; /* NOTREACHED */
 }
 
 I32
@@ -4336,8 +4335,8 @@ PP(pp_leaveeval)
                        SvPVX_const(namesv),
                         SvUTF8(namesv) ? -(I32)SvCUR(namesv) : (I32)SvCUR(namesv),
                        G_DISCARD);
-       retop = Perl_die(aTHX_ "%"SVf" did not return a true value",
-                              SVfARG(namesv));
+       Perl_die(aTHX_ "%"SVf" did not return a true value", SVfARG(namesv));
+        NOT_REACHED; /* NOTREACHED */
        /* die_unwind() did LEAVE, or we won't be here */
     }
     else {
index 998982d..f877fe2 100644 (file)
--- a/pp_pack.c
+++ b/pp_pack.c
@@ -448,7 +448,7 @@ S_measure_struct(pTHX_ tempsym_t* symptr)
          case e_star:
            Perl_croak(aTHX_ "Within []-length '*' not allowed in %s",
                         _action( symptr ) );
-            break;
+
          default:
            /* e_no_len and e_number */
            len = symptr->length;
@@ -567,7 +567,7 @@ S_group_end(pTHX_ const char *patptr, const char *patend, char ender)
     }
     Perl_croak(aTHX_ "No group ending character '%c' found in template",
                ender);
-    return 0;
+    NOT_REACHED; /* NOTREACHED */
 }
 
 
@@ -934,7 +934,7 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
            cuv = 0;
            cdouble = 0;
            continue;
-           break;
+
        case '(':
        {
             tempsym_t savsym = *symptr;
@@ -1060,7 +1060,7 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
            break;
        case '/':
            Perl_croak(aTHX_ "'/' must follow a numeric type in unpack");
-            break;
+
        case 'A':
        case 'Z':
        case 'a':
@@ -2083,6 +2083,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* symptr, SV **beglist, SV **endlist )
     bool found = next_symbol(symptr);
     bool utf8 = (symptr->flags & FLAG_PARSE_UTF8) ? 1 : 0;
     bool warn_utf8 = ckWARN(WARN_UTF8);
+    char* from;
 
     PERL_ARGS_ASSERT_PACK_REC;
 
@@ -2163,8 +2164,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* symptr, SV **beglist, SV **endlist )
                       (int) TYPE_NO_MODIFIERS(datumtype));
        case '%':
            Perl_croak(aTHX_ "'%%' may not be used in pack");
-       {
-           char *from;
+
        case '.' | TYPE_IS_SHRIEKING:
        case '.':
            if (howlen == e_star) from = start;
@@ -2213,7 +2213,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* symptr, SV **beglist, SV **endlist )
                goto shrink;
            }
            break;
-       }
+
        case '(': {
             tempsym_t savsym = *symptr;
            U32 group_modifiers = TYPE_MODIFIERS(datumtype & ~symptr->flags);
index 18b3d8e..ca6bbe7 100644 (file)
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -519,7 +519,8 @@ PP(pp_die)
            exsv = newSVpvs_flags("Died", SVs_TEMP);
        }
     }
-    return die_sv(exsv);
+    die_sv(exsv);
+    NOT_REACHED; /* NOTREACHED */
 }
 
 /* I/O. */
index 205c840..cafc670 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -9472,6 +9472,7 @@ S_reg(pTHX_ RExC_state_t *pRExC_state, I32 paren, I32 *flagp,U32 depth)
     bool is_open = 0;
     I32 freeze_paren = 0;
     I32 after_freeze = 0;
+    I32 num; /* numeric backreferences */
 
     char * parse_start = RExC_parse; /* MJD */
     char * const oregcomp_parse = RExC_parse;
@@ -9789,8 +9790,7 @@ S_reg(pTHX_ RExC_state_t *pRExC_state, I32 paren, I32 *flagp,U32 depth)
                nextchar(pRExC_state);
                return ret;
                /*notreached*/
-            { /* named and numeric backreferences */
-                I32 num;
+            /* named and numeric backreferences */
             case '&':            /* (?&NAME) */
                 parse_start = RExC_parse - 1;
               named_recursion:
@@ -9872,7 +9872,7 @@ S_reg(pTHX_ RExC_state_t *pRExC_state, I32 paren, I32 *flagp,U32 depth)
                 *flagp |= POSTPONED;
                 nextchar(pRExC_state);
                 return ret;
-            } /* named and numeric backreferences */
+
             assert(0); /* NOT REACHED */
 
            case '?':           /* (??...) */
@@ -11245,6 +11245,7 @@ S_regatom(pTHX_ RExC_state_t *pRExC_state, I32 *flagp, U32 depth)
     char *parse_start = RExC_parse;
     U8 op;
     int invert = 0;
+    U8 arg;
 
     GET_RE_DEBUG_FLAGS_DECL;
 
@@ -11361,7 +11362,6 @@ tryagain:
           literal text handling code.
        */
        switch ((U8)*++RExC_parse) {
-            U8 arg;
        /* Special Escapes */
        case 'A':
            RExC_seen_zerolen++;
index 0855bad..000653a 100644 (file)
--- a/regexec.c
+++ b/regexec.c
@@ -2258,7 +2258,6 @@ S_find_byclass(pTHX_ regexp * prog, const regnode *c, char *s,
         break;
     default:
         Perl_croak(aTHX_ "panic: unknown regstclass %d", (int)OP(c));
-        break;
     }
     return 0;
   got_it:
@@ -2455,7 +2454,6 @@ Perl_regexec_flags(pTHX_ REGEXP * const rx, char *stringarg, char *strend,
     /* Be paranoid... */
     if (prog == NULL || stringarg == NULL) {
        Perl_croak(aTHX_ "NULL regexp parameter");
-       return 0;
     }
 
     DEBUG_EXECUTE_r(
@@ -4618,7 +4616,6 @@ S_regmatch(pTHX_ regmatch_info *reginfo, char *startpos, regnode *prog)
                        break;
                    default:
                        Perl_croak(aTHX_ "panic: Unexpected FLAGS %u in op %u", FLAGS(scan), OP(scan));
-                       break;
                }
            }
            /* Note requires that all BOUNDs be lower than all NBOUNDs in
index db7ae8b..3295c4a 100644 (file)
--- a/regexp.h
+++ b/regexp.h
@@ -381,9 +381,9 @@ get_regex_charset_name(const U32 flags, STRLEN* const lenp)
        case REGEX_ASCII_MORE_RESTRICTED_CHARSET:
            *lenp = 2;
            return ASCII_MORE_RESTRICT_PAT_MODS;
-        default:
-           return "?";     /* Unknown */
     }
+    NOT_REACHED; /* NOTREACHED */
+    return "?";
 }
 
 /* Do we have some sort of anchor? */
diff --git a/sv.c b/sv.c
index 05f26d7..77c213e 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -4215,7 +4215,7 @@ Perl_sv_setsv_flags(pTHX_ SV *dstr, SV* sstr, const I32 flags)
        else
            Perl_croak(aTHX_ "Bizarre copy of %s", type);
        }
-       break;
+       NOT_REACHED; /* NOTREACHED */
 
     case SVt_REGEXP:
       upgregexp:
@@ -11346,10 +11346,10 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
            {
                char *ptr = ebuf + sizeof ebuf;
                bool tempalt = uv ? alt : FALSE; /* Vectors can't change alt */
+                unsigned dig;
                zeros = 0;
 
                switch (base) {
-                   unsigned dig;
                case 16:
                    p = (char *)((c == 'X') ? PL_hexdigit + 16 : PL_hexdigit);
                    do {
@@ -14677,7 +14677,7 @@ S_find_uninit_var(pTHX_ const OP *const obase, const SV *const uninit_sv,
            return varname(gv, '$', 0,
                    NULL, (I8)obase->op_private, FUV_SUBSCRIPT_ARRAY);
        }
-       break;
+       NOT_REACHED; /* NOTREACHED */
 
     case OP_EXISTS:
        o = cUNOPx(obase)->op_first;
@@ -14779,7 +14779,7 @@ S_find_uninit_var(pTHX_ const OP *const obase, const SV *const uninit_sv,
                ? '@' : '%',
                o->op_targ, NULL, 0, FUV_SUBSCRIPT_WITHIN);
        }
-       break;
+       NOT_REACHED; /* NOTREACHED */
     }
 
     case OP_AASSIGN:
diff --git a/toke.c b/toke.c
index 4d22549..e1a7694 100644 (file)
--- a/toke.c
+++ b/toke.c
@@ -5233,8 +5233,10 @@ Perl_yylex(pTHX)
            goto just_a_word_zero_gv;
        }
        s++;
+        {
+        OP *attrs;
+
        switch (PL_expect) {
-           OP *attrs;
        case XOPERATOR:
            if (!PL_in_my || PL_lex_state != LEX_NORMAL)
                break;
@@ -5374,6 +5376,7 @@ Perl_yylex(pTHX)
            }
            TOKEN(COLONATTR);
        }
+       }
        if (!PL_lex_allbrackets && PL_lex_fakeeof >= LEX_FAKEEOF_CLOSING) {
            s--;
            TOKEN(0);
diff --git a/utf8.c b/utf8.c
index f802199..7118226 100644 (file)
--- a/utf8.c
+++ b/utf8.c
@@ -2453,7 +2453,7 @@ Perl__core_swash_init(pTHX_ const char* pkg, const char* name, SV *listsv, I32 m
                Perl_croak(aTHX_
                           "Can't find Unicode property definition \"%"SVf"\"",
                           SVfARG(retval));
-           Perl_croak(aTHX_ "SWASHNEW didn't return an HV ref");
+                NOT_REACHED; /* NOTREACHED */
        }
     } /* End of calling the module to find the swash */