develooper Front page | perl.perl5.porters | Postings from November 2016

Re: [perl #129961] [PATCH] Remove some compiler hoops to jumpthrough for EBCDIC

Thread Previous | Thread Next
From:
Karl Williamson
Date:
November 26, 2016 22:33
Subject:
Re: [perl #129961] [PATCH] Remove some compiler hoops to jumpthrough for EBCDIC
Message ID:
acdbe00a-f41e-19f0-23db-f75c891b6c57@khwilliamson.com
On 10/24/2016 10:45 PM, Andy Lester (via RT) wrote:
> # New Ticket Created by  Andy Lester
> # Please include the string:  [perl #129961]
> # in the subject line of all future correspondence about this issue.
> # <URL: https://rt.perl.org/Ticket/Display.html?id=129961 >
>
>
> 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]
>
> This patch simplifies two bits of code that I came across while
> working on supporting the clang -Weverything flag.
>
> The first, in Perl_validate_proto, removes unnecessary variable
> initialization if proto of NULL is passed.
>
> The second, in S_scan_const, rearranges some code and #ifdefs so that
> the convert_unicode and real_range_max variables are only declared
> if EBCDIC is set.  This lets us no longer have to unnecessarily set
> useless variables to make the compiler happy, and it saves us from some
> unnecessary checks on "if (convert_unicode)".  One of the comments says
> "(Compilers should optimize this out for non-EBCDIC)", but now the
> compiler won't even see these unnecessary variables or tests.
>

Given that this code was generating compiler warnings was the reason to 
fix it.

But I do want to clarify that he reason I wrote it the way it was, was 
to cut down on the number of #ifdef EBCDICs there are.  Each added 
#ifdef is an added human maintenance cost.  It makes the code harder to 
read, as it disrupts the flow of what is going on.  It also means that 
some code won't even be compiled except under the right conditions, and 
that skipped code may not even be syntactically valid, which you won't 
find out until and unless there is some smoker (or worse, something from 
the field that we don't have the resources to reproduce) that has the 
correct configurations to trigger it.  When I first started the modern 
EBCDIC port, there were a bunch of syntax errors to work through from 
code that had been modified, but didn't get compiled.

By coding it the way it was, I made it easier to read, and hence 
maintain, and I knew it would compile on both types of machines.

> This diff may be easier to understand if it's viewed as two side-by-side
> files, such as with vimdiff.
>
> [Please do not change anything below this line]
> -----------------------------------------------------------------
> ---
> 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/local/sbin:/usr/local/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
>


Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About