Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Add/remove macros as uncovered by clang -Weverything #15737

Closed
p5pRT opened this issue Nov 28, 2016 · 15 comments
Closed

[PATCH] Add/remove macros as uncovered by clang -Weverything #15737

p5pRT opened this issue Nov 28, 2016 · 15 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 28, 2016

Migrated from rt.perl.org#130195 (status was 'resolved')

Searchable as RT130195$

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2016

From @petdance

Created by @petdance

clang -Weverything has uncovered some warnings​:

* Unreachable code
* Unused contexts in Perl_* functions
* Variables marked as unused that actually are used.

This patch cleans those up.

Perl Info

Flags:
    category=core
    severity=low
    Type=Patch
    PatchStatus=HasPatch

Site configuration information for perl 5.24.0:

Configured by andy at Sun Jun  5 23:28:46 CDT 2016.

Summary of my perl5 (revision 5 version 24 subversion 0) configuration:
   
  Platform:
    osname=linux, osvers=3.10.0-327.18.2.el7.x86_64, archname=x86_64-linux
    uname='linux clifford 3.10.0-327.18.2.el7.x86_64 #1 smp thu may 12 11:03:55 utc 2016 x86_64 x86_64 x86_64 gnulinux '
    config_args='-de -Dprefix=/home/andy/perl5/perlbrew/perls/perl-5.24.0 -Aeval:scriptdir=/home/andy/perl5/perlbrew/perls/perl-5.24.0/bin'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2',
    optimize='-O2',
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion='', gccversion='4.8.5 20150623 (Red Hat 4.8.5-4)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.17.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.17'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'

Locally applied patches:
    Devel::PatchPerl 1.38


@INC for perl 5.24.0:
    /home/andy/perl5/perlbrew/perls/perl-5.24.0/lib/site_perl/5.24.0/x86_64-linux
    /home/andy/perl5/perlbrew/perls/perl-5.24.0/lib/site_perl/5.24.0
    /home/andy/perl5/perlbrew/perls/perl-5.24.0/lib/5.24.0/x86_64-linux
    /home/andy/perl5/perlbrew/perls/perl-5.24.0/lib/5.24.0
    .


Environment for perl 5.24.0:
    HOME=/home/andy
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/andy/perl5/perlbrew/bin:/home/andy/perl5/perlbrew/perls/perl-5.24.0/bin:/home/andy/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
    PERLBREW_BASHRC_VERSION=0.75
    PERLBREW_HOME=/home/andy/.perlbrew
    PERLBREW_MANPATH=/home/andy/perl5/perlbrew/perls/perl-5.24.0/man
    PERLBREW_PATH=/home/andy/perl5/perlbrew/bin:/home/andy/perl5/perlbrew/perls/perl-5.24.0/bin
    PERLBREW_PERL=perl-5.24.0
    PERLBREW_ROOT=/home/andy/perl5/perlbrew
    PERLBREW_VERSION=0.75
    PERLCRITIC=/home/andy/tw/Dev/perlcriticrc
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2016

From @petdance

macros.patch
diff --git a/hv.c b/hv.c
index c2646ca..7239892 100644
--- a/hv.c
+++ b/hv.c
@@ -2058,6 +2058,7 @@ Perl_hv_fill(pTHX_ HV *const hv)
     STRLEN count = 0;
     HE **ents = HvARRAY(hv);
 
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_HV_FILL;
 
     /* No keys implies no buckets used.
diff --git a/mathoms.c b/mathoms.c
index a3f20e7..c74a386 100644
--- a/mathoms.c
+++ b/mathoms.c
@@ -1688,6 +1688,7 @@ Perl_is_utf8_char_buf(const U8 *buf, const U8* buf_end)
 UV
 Perl_valid_utf8_to_uvuni(pTHX_ const U8 *s, STRLEN *retlen)
 {
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_VALID_UTF8_TO_UVUNI;
 
     return NATIVE_TO_UNI(valid_utf8_to_uvchr(s, retlen));
@@ -1750,6 +1751,7 @@ See L</utf8n_to_uvchr> for details on when the REPLACEMENT CHARACTER is returned
 UV
 Perl_utf8_to_uvuni(pTHX_ const U8 *s, STRLEN *retlen)
 {
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_UTF8_TO_UVUNI;
 
     return NATIVE_TO_UNI(valid_utf8_to_uvchr(s, retlen));
diff --git a/op.c b/op.c
index 3cd7ea2..b3b7009 100644
--- a/op.c
+++ b/op.c
@@ -10360,11 +10360,13 @@ Perl_ck_defined(pTHX_ OP *o)		/* 19990527 MJD */
 	case OP_PADAV:
 	    Perl_croak(aTHX_ "Can't use 'defined(@array)'"
 			     " (Maybe you should just omit the defined()?)");
-	break;
+            NOT_REACHED;
+            break;
 	case OP_RV2HV:
 	case OP_PADHV:
 	    Perl_croak(aTHX_ "Can't use 'defined(%%hash)'"
 			     " (Maybe you should just omit the defined()?)");
+            NOT_REACHED;
 	    break;
 	default:
 	    /* no warning */
@@ -14459,7 +14461,7 @@ Perl_rpeep(pTHX_ OP *o)
             oldop    = ourlast;
             o        = oldop->op_next;
             goto redo;
-	    
+            NOT_REACHED;
 	    break;
 	}
 
diff --git a/perlio.c b/perlio.c
index 0ca6dde..ad1c6fe 100644
--- a/perlio.c
+++ b/perlio.c
@@ -848,7 +848,6 @@ XS(XS_PerlIO__Layer__NoWarnings)
        during loading of layers.
      */
     dXSARGS;
-    PERL_UNUSED_ARG(cv);
     PERL_UNUSED_VAR(items);
     DEBUG_i(
         if (items)
@@ -860,7 +859,6 @@ XS(XS_PerlIO__Layer__find); /* prototype to pass -Wmissing-prototypes */
 XS(XS_PerlIO__Layer__find)
 {
     dXSARGS;
-    PERL_UNUSED_ARG(cv);
     if (items < 2)
 	Perl_croak(aTHX_ "Usage class->find(name[,load])");
     else {
@@ -3559,6 +3557,7 @@ STDCHAR *
 PerlIOStdio_get_base(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return (STDCHAR*)PerlSIO_get_base(stdio);
 }
 
@@ -3566,6 +3565,7 @@ Size_t
 PerlIOStdio_get_bufsiz(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return PerlSIO_get_bufsiz(stdio);
 }
 #endif
@@ -3575,6 +3575,7 @@ STDCHAR *
 PerlIOStdio_get_ptr(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return (STDCHAR*)PerlSIO_get_ptr(stdio);
 }
 
@@ -3582,6 +3583,7 @@ SSize_t
 PerlIOStdio_get_cnt(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return PerlSIO_get_cnt(stdio);
 }
 
@@ -3589,6 +3591,7 @@ void
 PerlIOStdio_set_ptrcnt(pTHX_ PerlIO *f, STDCHAR * ptr, SSize_t cnt)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     if (ptr != NULL) {
 #ifdef STDIO_PTR_LVALUE
         /* This is a long-standing infamous mess.  The root of the
@@ -5104,6 +5107,7 @@ PerlIO_tmpfile(void)
 void
 Perl_PerlIO_save_errno(pTHX_ PerlIO *f)
 {
+    PERL_UNUSED_CONTEXT;
     if (!PerlIOValid(f))
 	return;
     PerlIOBase(f)->err = errno;
@@ -5119,6 +5123,7 @@ Perl_PerlIO_save_errno(pTHX_ PerlIO *f)
 void
 Perl_PerlIO_restore_errno(pTHX_ PerlIO *f)
 {
+    PERL_UNUSED_CONTEXT;
     if (!PerlIOValid(f))
 	return;
     SETERRNO(PerlIOBase(f)->err, PerlIOBase(f)->os_err);
diff --git a/universal.c b/universal.c
index 06f595f..95934ca 100644
--- a/universal.c
+++ b/universal.c
@@ -554,7 +554,6 @@ XS(XS_Internals_SvREADONLY)	/* This is dangerous stuff. */
     dXSARGS;
     SV * const svz = ST(0);
     SV * sv;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if (!SvROK(svz))
@@ -588,7 +587,6 @@ XS(XS_constant__make_const)	/* This is dangerous stuff. */
     dXSARGS;
     SV * const svz = ST(0);
     SV * sv;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if (!SvROK(svz) || items != 1)
@@ -616,7 +614,6 @@ XS(XS_Internals_SvREFCNT)	/* This is dangerous stuff. */
     SV * const svz = ST(0);
     SV * sv;
     U32 refcnt;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if ((items != 1 && items != 2) || !SvROK(svz))
@@ -777,7 +774,6 @@ XS(XS_re_is_regexp); /* prototype to pass -Wmissing-prototypes */
 XS(XS_re_is_regexp)
 {
     dXSARGS;
-    PERL_UNUSED_VAR(cv);
 
     if (items != 1)
 	croak_xs_usage(cv, "sv");

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

From @jkeenan

On Mon, 28 Nov 2016 05​:08​:04 GMT, petdance wrote​:

This is a bug report for perl from andy@​petdance.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

-----------------------------------------------------------------
[Please describe your issue here]

clang -Weverything has uncovered some warnings​:

* Unreachable code
* Unused contexts in Perl_* functions
* Variables marked as unused that actually are used.

This patch cleans those up.

1. Could you send the exact command you used for './Configure'? The 'perl -V' output submitted suggests the patch was created with 'gcc'. For a good comparison, I'd like to configure the same way both in blead and in a branch to which your branch has been applied.

2. Are you concerned with warnings only in 'make' -- or in './Configure' as well?

3. Smoke branch​: smoke-me/jkeenan/petdance/130195-macros

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

From @jkeenan

On Sat, 03 Dec 2016 03​:35​:00 GMT, jkeenan wrote​:

On Mon, 28 Nov 2016 05​:08​:04 GMT, petdance wrote​:

This is a bug report for perl from andy@​petdance.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

-----------------------------------------------------------------
[Please describe your issue here]

clang -Weverything has uncovered some warnings​:

* Unreachable code
* Unused contexts in Perl_* functions
* Variables marked as unused that actually are used.

This patch cleans those up.

1. Could you send the exact command you used for './Configure'? The
'perl -V' output submitted suggests the patch was created with 'gcc'.
For a good comparison, I'd like to configure the same way both in
blead and in a branch to which your branch has been applied.

2. Are you concerned with warnings only in 'make' -- or in
'./Configure' as well?

3. Smoke branch​: smoke-me/jkeenan/petdance/130195-macros

Thank you very much.

Configuring both blead and branch on Linux with​:

#####
sh ./Configure -des -Dusedevel -Dcc="clang -Weverything"
#####

... followed by transcribing the output of 'make test_prep' in each to a .diff file and then searching for 'warning​:', I got​:

38837 blead

38621 branch

Is that reduction of 216 warnings in line with your expectations?

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

From @jkeenan

On Sat, 03 Dec 2016 03​:40​:16 GMT, jkeenan wrote​:

On Sat, 03 Dec 2016 03​:35​:00 GMT, jkeenan wrote​:

On Mon, 28 Nov 2016 05​:08​:04 GMT, petdance wrote​:

This is a bug report for perl from andy@​petdance.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

-----------------------------------------------------------------
[Please describe your issue here]

clang -Weverything has uncovered some warnings​:

* Unreachable code
* Unused contexts in Perl_* functions
* Variables marked as unused that actually are used.

This patch cleans those up.

1. Could you send the exact command you used for './Configure'? The
'perl -V' output submitted suggests the patch was created with 'gcc'.
For a good comparison, I'd like to configure the same way both in
blead and in a branch to which your branch has been applied.

2. Are you concerned with warnings only in 'make' -- or in
'./Configure' as well?

3. Smoke branch​: smoke-me/jkeenan/petdance/130195-macros

Thank you very much.

Configuring both blead and branch on Linux with​:

#####
sh ./Configure -des -Dusedevel -Dcc="clang -Weverything"
#####

... followed by transcribing the output of 'make test_prep' in each to
a .diff file and then searching for 'warning​:', I got​:

38837 blead

38621 branch

Is that reduction of 216 warnings in line with your expectations?

I also tested blead and branch on FreeBSD-11.0. On this platform I configured with​:

#####
sh ./Configure -des -Dusedevel -Duseithreads -Doptimize="-O2 -pipe -fstack-protextor -fno-strict-aliasing" -Dcc="clang -Weverything"
#####

Here the branch showed little improvement over blead​:

111004 blead

111002 branch

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

From @petdance

I'm sorry, I didn't mean to make it sound like the warnings only apply to clang. I'm just saying that clang discovered the warnings. I discovered this on my work branch trying to get Perl to build clean under clang -Weverything.

clang -Weverything on its own with no modifications generates a huge number of warnings. We will not be using -Weverything without other -Wno-xxxx flags to quiet them down.

It sounds exactly right that only two warnings were squelched by this patch.

The flags I'm currently using are

# -Wno-unused-macros​: We have tons of macros and it's OK if we don't use them.
# -Wno-gnu-statement-expression​: Commonly used all over the codebase.
# -Wno-shadow​: We have many common global variable names. We would have to change many local variables.
# -Wno-overlength-strings​: Compiler limit we don't care about
# -Wno-disabled-macro-expansion
# -Wno-variadic-macros​: We use them.
# -Wno-cast-align
# -Wno-extended-offsetof
# -Wno-c11-extensions
# -Wno-format-nonliteral
# -Wno-switch-enum​: This one is not mollified by a default in the case. https://stackoverflow.com/questions/16631713/
# -Wno-conversion​: We have far too many conversions to clean up
# -Wno-missing-variable-declarations​: globals.c barfs

You can also look at the Weverything branch at https://github.com/petdance/perl5 if you want details.

@p5pRT
Copy link
Author

p5pRT commented Dec 3, 2016

From @jkeenan

On Sat, 03 Dec 2016 18​:43​:31 GMT, petdance wrote​:

I'm sorry, I didn't mean to make it sound like the warnings only apply
to clang. I'm just saying that clang discovered the warnings. I
discovered this on my work branch trying to get Perl to build clean
under clang -Weverything.

clang -Weverything on its own with no modifications generates a huge
number of warnings. We will not be using -Weverything without other
-Wno-xxxx flags to quiet them down.

It sounds exactly right that only two warnings were squelched by this
patch.

The flags I'm currently using are

# -Wno-unused-macros​: We have tons of macros and it's OK if we don't
use them.
# -Wno-gnu-statement-expression​: Commonly used all over the codebase.
# -Wno-shadow​: We have many common global variable names. We would
have to change many local variables.
# -Wno-overlength-strings​: Compiler limit we don't care about
# -Wno-disabled-macro-expansion
# -Wno-variadic-macros​: We use them.
# -Wno-cast-align
# -Wno-extended-offsetof
# -Wno-c11-extensions
# -Wno-format-nonliteral
# -Wno-switch-enum​: This one is not mollified by a default in the
case. https://stackoverflow.com/questions/16631713/
# -Wno-conversion​: We have far too many conversions to clean up
# -Wno-missing-variable-declarations​: globals.c barfs

You can also look at the Weverything branch at
https://github.com/petdance/perl5 if you want details.

That's fine. I'd still appreciate the information requested in items 1 and 2 in https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130195#txn-1437999

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2016

From @petdance

2. Are you concerned with warnings only in 'make' -- or in './Configure' as well?

Just in the make.

./Configure \
  -des \
  -Dusedevel \
  -Dusethreads \
  -Duselongdouble \
  -Doptimize="-O3" \
  -Acc=clang \
  -Accflags="-DDEBUGGING" \
  -Dprefix=/var/perl/blead \
  -Dmydomain=.petdance.com \
  -Dcf_email=andy@​petdance.com \
  -Dperladmin=andy@​petdance.com \

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2016

From @jkeenan

On Sun, 04 Dec 2016 04​:46​:58 GMT, petdance wrote​:

2. Are you concerned with warnings only in 'make' -- or in
'./Configure' as well?

Just in the make.

./Configure \
-des \
-Dusedevel \
-Dusethreads \
-Duselongdouble \
-Doptimize="-O3" \
-Acc=clang \
-Accflags="-DDEBUGGING" \
-Dprefix=/var/perl/blead \
-Dmydomain=.petdance.com \
-Dcf_email=andy@​petdance.com \
-Dperladmin=andy@​petdance.com \

A little bit off-topic (but helpful in interpreting my 'make' output)​:

Why in the above do you say​:

#####
-Acc=clang -Accflags="-DDEBUGGING"
#####

... rather than​:

#####
-Dcc=clang -DDEBUGGING
#####

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2016

From @petdance

-Acc=clang -Accflags="-DDEBUGGING"
... rather than​:
-Dcc=clang -DDEBUGGING

No significant reason that I'm aware of.

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2016

From @jkeenan

On Sun, 04 Dec 2016 17​:18​:53 GMT, petdance wrote​:

-Acc=clang -Accflags="-DDEBUGGING"
... rather than​:
-Dcc=clang -DDEBUGGING

No significant reason that I'm aware of.

Okay. +1 to the revision. Since this is internals code, I'll defer to Dave M or Tony C to apply it. I am attaching the 'git format-patch' version of the patch.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 4, 2016

From @jkeenan

130195-0001-Clean-up-warnings-uncovered-by-clang-Weverything.patch
From b64630a504e953e8d02507a11e14f5fb8eb0aae9 Mon Sep 17 00:00:00 2001
From: Andy Lester <andy@petdance.com>
Date: Fri, 2 Dec 2016 22:07:26 -0500
Subject: [PATCH] Clean up warnings uncovered by 'clang -Weverything'.

For: RT #130195
---
 hv.c        | 1 +
 mathoms.c   | 2 ++
 op.c        | 6 ++++--
 perlio.c    | 9 +++++++--
 universal.c | 4 ----
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/hv.c b/hv.c
index c2646ca..7239892 100644
--- a/hv.c
+++ b/hv.c
@@ -2058,6 +2058,7 @@ Perl_hv_fill(pTHX_ HV *const hv)
     STRLEN count = 0;
     HE **ents = HvARRAY(hv);
 
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_HV_FILL;
 
     /* No keys implies no buckets used.
diff --git a/mathoms.c b/mathoms.c
index a3f20e7..c74a386 100644
--- a/mathoms.c
+++ b/mathoms.c
@@ -1688,6 +1688,7 @@ Perl_is_utf8_char_buf(const U8 *buf, const U8* buf_end)
 UV
 Perl_valid_utf8_to_uvuni(pTHX_ const U8 *s, STRLEN *retlen)
 {
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_VALID_UTF8_TO_UVUNI;
 
     return NATIVE_TO_UNI(valid_utf8_to_uvchr(s, retlen));
@@ -1750,6 +1751,7 @@ See L</utf8n_to_uvchr> for details on when the REPLACEMENT CHARACTER is returned
 UV
 Perl_utf8_to_uvuni(pTHX_ const U8 *s, STRLEN *retlen)
 {
+    PERL_UNUSED_CONTEXT;
     PERL_ARGS_ASSERT_UTF8_TO_UVUNI;
 
     return NATIVE_TO_UNI(valid_utf8_to_uvchr(s, retlen));
diff --git a/op.c b/op.c
index 3cd7ea2..b3b7009 100644
--- a/op.c
+++ b/op.c
@@ -10360,11 +10360,13 @@ Perl_ck_defined(pTHX_ OP *o)		/* 19990527 MJD */
 	case OP_PADAV:
 	    Perl_croak(aTHX_ "Can't use 'defined(@array)'"
 			     " (Maybe you should just omit the defined()?)");
-	break;
+            NOT_REACHED;
+            break;
 	case OP_RV2HV:
 	case OP_PADHV:
 	    Perl_croak(aTHX_ "Can't use 'defined(%%hash)'"
 			     " (Maybe you should just omit the defined()?)");
+            NOT_REACHED;
 	    break;
 	default:
 	    /* no warning */
@@ -14459,7 +14461,7 @@ Perl_rpeep(pTHX_ OP *o)
             oldop    = ourlast;
             o        = oldop->op_next;
             goto redo;
-	    
+            NOT_REACHED;
 	    break;
 	}
 
diff --git a/perlio.c b/perlio.c
index 0ca6dde..ad1c6fe 100644
--- a/perlio.c
+++ b/perlio.c
@@ -848,7 +848,6 @@ XS(XS_PerlIO__Layer__NoWarnings)
        during loading of layers.
      */
     dXSARGS;
-    PERL_UNUSED_ARG(cv);
     PERL_UNUSED_VAR(items);
     DEBUG_i(
         if (items)
@@ -860,7 +859,6 @@ XS(XS_PerlIO__Layer__find); /* prototype to pass -Wmissing-prototypes */
 XS(XS_PerlIO__Layer__find)
 {
     dXSARGS;
-    PERL_UNUSED_ARG(cv);
     if (items < 2)
 	Perl_croak(aTHX_ "Usage class->find(name[,load])");
     else {
@@ -3559,6 +3557,7 @@ STDCHAR *
 PerlIOStdio_get_base(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return (STDCHAR*)PerlSIO_get_base(stdio);
 }
 
@@ -3566,6 +3565,7 @@ Size_t
 PerlIOStdio_get_bufsiz(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return PerlSIO_get_bufsiz(stdio);
 }
 #endif
@@ -3575,6 +3575,7 @@ STDCHAR *
 PerlIOStdio_get_ptr(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return (STDCHAR*)PerlSIO_get_ptr(stdio);
 }
 
@@ -3582,6 +3583,7 @@ SSize_t
 PerlIOStdio_get_cnt(pTHX_ PerlIO *f)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     return PerlSIO_get_cnt(stdio);
 }
 
@@ -3589,6 +3591,7 @@ void
 PerlIOStdio_set_ptrcnt(pTHX_ PerlIO *f, STDCHAR * ptr, SSize_t cnt)
 {
     FILE * const stdio = PerlIOSelf(f, PerlIOStdio)->stdio;
+    PERL_UNUSED_CONTEXT;
     if (ptr != NULL) {
 #ifdef STDIO_PTR_LVALUE
         /* This is a long-standing infamous mess.  The root of the
@@ -5104,6 +5107,7 @@ PerlIO_tmpfile(void)
 void
 Perl_PerlIO_save_errno(pTHX_ PerlIO *f)
 {
+    PERL_UNUSED_CONTEXT;
     if (!PerlIOValid(f))
 	return;
     PerlIOBase(f)->err = errno;
@@ -5119,6 +5123,7 @@ Perl_PerlIO_save_errno(pTHX_ PerlIO *f)
 void
 Perl_PerlIO_restore_errno(pTHX_ PerlIO *f)
 {
+    PERL_UNUSED_CONTEXT;
     if (!PerlIOValid(f))
 	return;
     SETERRNO(PerlIOBase(f)->err, PerlIOBase(f)->os_err);
diff --git a/universal.c b/universal.c
index 06f595f..95934ca 100644
--- a/universal.c
+++ b/universal.c
@@ -554,7 +554,6 @@ XS(XS_Internals_SvREADONLY)	/* This is dangerous stuff. */
     dXSARGS;
     SV * const svz = ST(0);
     SV * sv;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if (!SvROK(svz))
@@ -588,7 +587,6 @@ XS(XS_constant__make_const)	/* This is dangerous stuff. */
     dXSARGS;
     SV * const svz = ST(0);
     SV * sv;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if (!SvROK(svz) || items != 1)
@@ -616,7 +614,6 @@ XS(XS_Internals_SvREFCNT)	/* This is dangerous stuff. */
     SV * const svz = ST(0);
     SV * sv;
     U32 refcnt;
-    PERL_UNUSED_ARG(cv);
 
     /* [perl #77776] - called as &foo() not foo() */
     if ((items != 1 && items != 2) || !SvROK(svz))
@@ -777,7 +774,6 @@ XS(XS_re_is_regexp); /* prototype to pass -Wmissing-prototypes */
 XS(XS_re_is_regexp)
 {
     dXSARGS;
-    PERL_UNUSED_VAR(cv);
 
     if (items != 1)
 	croak_xs_usage(cv, "sv");
-- 
2.7.4

@p5pRT
Copy link
Author

p5pRT commented Dec 5, 2016

From @iabyn

On Sun, Dec 04, 2016 at 01​:17​:51PM -0800, James E Keenan via RT wrote​:

On Sun, 04 Dec 2016 17​:18​:53 GMT, petdance wrote​:

-Acc=clang -Accflags="-DDEBUGGING"
... rather than​:
-Dcc=clang -DDEBUGGING

No significant reason that I'm aware of.

Okay. +1 to the revision. Since this is internals code, I'll defer to Dave M or Tony C to apply it. I am attaching the 'git format-patch' version of the patch.

I've applied it as v5.25.7-63-ge9b8343.

Arguably all the 'PERL_UNUSED_CONTEXT;'s are an indication that maybe
APIs needs changing, but I didn't want to go there.

I also added the following followup-commit​:

commit 62d9081
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Mon Dec 5 12​:47​:42 2016 +0000
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Mon Dec 5 12​:47​:42 2016 +0000

  add some /* NOTREACHED */
 
  The previous commit added some
 
  NOT_REACHED;
 
  but these days the correct voodoo is, apparently,
 
  NOT_REACHED; /* NOTREACHED */
 
  to please as many compilers and static analysers as possible

--
You're only as old as you look.

@p5pRT
Copy link
Author

p5pRT commented Dec 5, 2016

@iabyn - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant