Skip Menu |
Report information
Id: 130087
Status: pending release
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: todd [at] rinaldo.us
Cc:
AdminCc:

Operating System: darwin
PatchStatus: (no value)
Severity: low
Type: library
Perl Version: 5.18.2
Fixed In: (no value)



Date: Sun, 13 Nov 2016 17:59:42 +0100
To: perlbug [...] perl.org
Subject: Reduce module dependencies of B and O to simplify compiler second guessing.
From: Todd Rinaldo <todd [...] rinaldo.us>
Download (untitled) / with headers
text/plain 3.2k
This is a bug report for perl from toddr@cpan.org, generated with the help of perlbug 1.39 running under perl 5.18.2. ----------------------------------------------------------------- [Please describe your issue here] While working with B::C, it is impossible to tell what modules are loaded by the program or by O/B. As a result, it's ideal if B/O do not load modules if they can help it. I'm submitting a proposed patch to address this. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=library severity=low module=O --- Site configuration information for perl 5.18.2: Configured by root at Tue Aug 11 04:03:09 PDT 2015. Summary of my perl5 (revision 5 version 18 subversion 2) configuration: Platform: osname=darwin, osvers=15.0, archname=darwin-thread-multi-2level uname='darwin osx219.apple.com 15.0 darwin kernel version 15.0.0: fri may 22 22:03:51 pdt 2015; root:xnu-3216.0.0.1.11~1development_x86_64 x86_64 ' config_args='-ds -e -Dprefix=/usr -Dccflags=-g -pipe -Dldflags= -Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none -Dcc=cc' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-arch i386 -arch x86_64 -g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing -fstack-protector', optimize='-Os', cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing -fstack-protector' ccversion='', gccversion='4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.0.59.1)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc -mmacosx-version-min=10.11.3', ldflags ='-arch i386 -arch x86_64 -fstack-protector' libpth=/usr/lib /usr/local/lib libs= perllibs= libc=, so=dylib, useshrplib=true, libperl=libperl.dylib gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-arch i386 -arch x86_64 -bundle -undefined dynamic_lookup -fstack-protector' Locally applied patches: /Library/Perl/Updates/<version> comes before system perl directories installprivlib and installarchlib points to the Updates directory --- @INC for perl 5.18.2: /Library/Perl/5.18/darwin-thread-multi-2level /Library/Perl/5.18 /Network/Library/Perl/5.18/darwin-thread-multi-2level /Network/Library/Perl/5.18 /Library/Perl/Updates/5.18.2 /System/Library/Perl/5.18/darwin-thread-multi-2level /System/Library/Perl/5.18 /System/Library/Perl/Extras/5.18/darwin-thread-multi-2level /System/Library/Perl/Extras/5.18 . --- Environment for perl 5.18.2: DYLD_LIBRARY_PATH (unset) HOME=/Users/toddr LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=:/sw/bin PERLDB_OPTS=RemotePort=:9000 PERL_BADLANG (unset) SHELL=/bin/zsh
RT-Send-CC: perl5-porters [...] perl.org
Patch attached.
Subject: 0001-Remove-unnecessary-module-loads-from-B-and-O.patch

Message body is not shown because it is too large.

Subject: Re: [perl #130087] Reduce module dependencies of B and O to simplify compiler second guessing.
To: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Date: Mon, 5 Jun 2017 17:12:19 +0100
Download (untitled) / with headers
text/plain 798b
On Sun, Nov 13, 2016 at 09:00:33AM -0800, Todd Rinaldo wrote: Show quoted text
> While working with B::C, it is impossible to tell what modules are > loaded by the program or by O/B. As a result, it's ideal if B/O do not > load modules if they can help it. I'm submitting a proposed patch to > address this.
Sorry, this seems to have gotten overlooked, and no longer cleanly applies. I'm happy with the patch, except for removing strict from B.pm. If you really, really need to avoid loading strict.pm, then perhaps replace use strict; with something like BEGIN { $^H = 0x6e2 }; Then have a test somewhere which asserts somehow that the values of %bitmask and %explicit_bitmask in strict.pm 'or' to 0x6e2. -- In economics, the exam questions are the same every year. They just change the answers.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 462b
On Mon, 05 Jun 2017 09:12:57 -0700, davem wrote: Show quoted text
> I'm happy with the patch, except for removing strict from B.pm. > If you really, really need to avoid loading strict.pm, then perhaps > replace > > use strict; > > with something like > > BEGIN { $^H = 0x6e2 }; > > Then have a test somewhere which asserts somehow that the values of > %bitmask and %explicit_bitmask in strict.pm 'or' to 0x6e2. >
Thanks, I'll update and re-submit the patch soon.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 777b
Show quoted text
> Sorry, this seems to have gotten overlooked, and no longer cleanly > applies. > > I'm happy with the patch, except for removing strict from B.pm. > If you really, really need to avoid loading strict.pm, then perhaps > replace > > use strict; > > with something like > > BEGIN { $^H = 0x6e2 }; > > Then have a test somewhere which asserts somehow that the values of > %bitmask and %explicit_bitmask in strict.pm 'or' to 0x6e2. >
Dave, How about a unit test instead to assure both warnings and strict are met? I had to do some fixups to meet that requirement. See attached patches. If this doesn't meet all of your needs, then I have concerns about your $^H value. I'm getting this? $>perl -e'BEGIN {use strict; use warnings; print $^H . "\n"}' 256 Todd
Subject: 0001-Remove-unnecessary-module-loads-from-B-and-O.patch

Message body is not shown because it is too large.

Subject: 0002-Fix-warnings-used-once-for-B.pm.patch
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Todd Rinaldo <toddr@cpan.org> Date: Wed, 13 Sep 2017 14:35:30 -0500 Subject: [PATCH 2/4] Fix warnings (used once) for B.pm --- ext/B/B.pm | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ext/B/B.pm b/ext/B/B.pm index 47c43536dc..9ffebbda82 100644 --- a/ext/B/B.pm +++ b/ext/B/B.pm @@ -79,12 +79,12 @@ push @B::EXPORT_OK, (qw(minus_c ppname save_BEGINs @B::SPECIAL::ISA = 'B::OBJECT'; -@B::optype = qw(OP UNOP BINOP LOGOP LISTOP PMOP SVOP PADOP PVOP LOOP COP +our @optype = qw(OP UNOP BINOP LOGOP LISTOP PMOP SVOP PADOP PVOP LOOP COP METHOP UNOP_AUX); # bytecode.pl contained the following comment: # Nullsv *must* come first in the following so that the condition # ($$sv == 0) can continue to be used to test (sv == Nullsv). -@B::specialsv_name = qw(Nullsv &PL_sv_undef &PL_sv_yes &PL_sv_no +our @specialsv_name = qw(Nullsv &PL_sv_undef &PL_sv_yes &PL_sv_no (SV*)pWARN_ALL (SV*)pWARN_NONE (SV*)pWARN_STD &PL_sv_zero); @@ -120,15 +120,17 @@ sub B::IV::int_value { } sub B::NULL::as_string() {""} -*B::IV::as_string = \*B::IV::int_value; -*B::PV::as_string = \*B::PV::PV; +*B::IV::as_string = *B::IV::as_string = \*B::IV::int_value; +*B::PV::as_string = *B::PV::as_string = \*B::PV::PV; # The input typemap checking makes no distinction between different SV types, # so the XS body will generate the same C code, despite the different XS # "types". So there is no change in behaviour from doing "newXS" like this, # compared with the old approach of having a (near) duplicate XS body. # We should fix the typemap checking. -*B::IV::RV = \*B::PV::RV if $] > 5.012; + +# Since perl 5.12.0 +*B::IV::RV = *B::IV::RV = \*B::PV::RV; my $debug; my $op_count = 0;
Subject: 0003-Fix-warnings-used-once-for-O.pm.patch
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Todd Rinaldo <toddr@cpan.org> Date: Wed, 13 Sep 2017 14:35:35 -0500 Subject: [PATCH 3/4] Fix warnings (used once) for O.pm --- ext/B/O.pm | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ext/B/O.pm b/ext/B/O.pm index 7393441b05..09a905eaee 100644 --- a/ext/B/O.pm +++ b/ext/B/O.pm @@ -4,12 +4,15 @@ our $VERSION = '1.02'; use B (); +our $BEGIN_output; +our $saveout_fh; + sub import { my ($class, @options) = @_; my ($quiet, $veryquiet) = (0, 0); if ($options[0] eq '-q' || $options[0] eq '-qq') { $quiet = 1; - open (SAVEOUT, ">&STDOUT"); + open ($saveout_fh, ">&", STDOUT); close STDOUT; open (STDOUT, ">", \$O::BEGIN_output); if ($options[0] eq '-qq') { @@ -27,8 +30,8 @@ sub import { CHECK { if ($quiet) { close STDOUT; - open (STDOUT, ">&SAVEOUT"); - close SAVEOUT; + open (STDOUT, ">&", $saveout_fh); + close $saveout_fh; } # Note: if you change the code after this 'use', please
Subject: 0004-ext-B-t-strict.t-Assure-B.pm-and-O.pm-pass-strict-an.patch
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Todd Rinaldo <toddr@cpan.org> Date: Wed, 13 Sep 2017 14:35:39 -0500 Subject: [PATCH 4/4] ext/B/t/strict.t: Assure B.pm and O.pm pass strict and warnings checks. We need to test these in unit tests since they do not load these modules to prevent pollution of the stash in compilers. --- MANIFEST | 1 + ext/B/t/strict.t | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 ext/B/t/strict.t diff --git a/MANIFEST b/MANIFEST index ad24a2d28b..5bcafdc8c2 100644 --- a/MANIFEST +++ b/MANIFEST @@ -3915,6 +3915,7 @@ ext/B/t/optree_varinit.t my,our,local var init optimization ext/B/t/OptreeCheck.pm optree comparison tool ext/B/t/pragma.t See if user pragmas work. ext/B/t/showlex.t See if B::ShowLex works +ext/B/t/strict.t See if B works with strict and warnings. ext/B/t/sv_stash.t See if SvSTASH() works ext/B/t/terse.t See if B::Terse works ext/B/t/walkoptree.t See if B::walkoptree (and friends) work diff --git a/ext/B/t/strict.t b/ext/B/t/strict.t new file mode 100644 index 0000000000..74693d4d3e --- /dev/null +++ b/ext/B/t/strict.t @@ -0,0 +1,28 @@ +#!./perl -w + +BEGIN { + unshift @INC, 't'; + require Config; + if ( ( $Config::Config{'extensions'} !~ /\bB\b/ ) ) { + print "1..0 # Skip -- Perl configured without B module\n"; + exit 0; + } + require '../../t/test.pl'; +} + +use strict; +use warnings; + +use B (); +use O (); + +plan tests => 2; + +foreach my $module (qw/B O/) { + my $path = $INC{ $module . '.pm' }; + my $check = "$^X -cw -Mstrict $path 2>&1"; + my $got = `$check`; + is( $got, "$path syntax OK\n", "$module.pm compiles without errors" ) + or diag($got); +} +
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130087] Reduce module dependencies of B and O to simplify compiler second guessing.
Date: Wed, 13 Sep 2017 21:31:20 +0100
Download (untitled) / with headers
text/plain 1.1k
Todd Rinaldo via RT wrote: Show quoted text
>$>perl -e'BEGIN {use strict; use warnings; print $^H . "\n"}' >256
That's showing $^H at *runtime* of the code that has the pragmata in effect. $^H reflects the pragmata in effect at the current point of compilation, which in this case is no pragmata. To get the hint bits corresponding to the strict pragma, you need to look at $^H during *compilation* of code with the pragma in effect: $ perl -lwe 'use strict; BEGIN { printf "%x\n", $^H }' 7e2 You could also pull hint bits from perl.h: #define HINT_STRICT_REFS 0x00000002 /* strict pragma */ #define HINT_EXPLICIT_STRICT_REFS 0x00000020 /* strict.pm */ #define HINT_EXPLICIT_STRICT_SUBS 0x00000040 /* strict.pm */ #define HINT_EXPLICIT_STRICT_VARS 0x00000080 /* strict.pm */ #define HINT_BLOCK_SCOPE 0x00000100 #define HINT_STRICT_SUBS 0x00000200 /* strict pragma */ #define HINT_STRICT_VARS 0x00000400 /* strict pragma */ The warnings pragma doesn't affect $^H, but rather ${^WARNING_BITS}. Similar compile-time/runtime considerations apply to it. However, the bit assignment there isn't as stable as the hint bit assignments. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 907b
On Wed, 13 Sep 2017 13:21:53 -0700, TODDR wrote: Show quoted text
> > Sorry, this seems to have gotten overlooked, and no longer cleanly > > applies. > > > > I'm happy with the patch, except for removing strict from B.pm. > > If you really, really need to avoid loading strict.pm, then perhaps > > replace > > > > use strict; > > > > with something like > > > > BEGIN { $^H = 0x6e2 }; > > > > Then have a test somewhere which asserts somehow that the values of > > %bitmask and %explicit_bitmask in strict.pm 'or' to 0x6e2. > >
> > Dave, > > How about a unit test instead to assure both warnings and strict are > met? I had to do some fixups to meet that requirement. > > See attached patches.
+ } + require '../../t/test.pl'; +} + +use strict; +use warnings; + +use B (); Why use test.pl here instead of Test::More - the code doesn't use any of the special functions (like runperl()) that test.pl provides. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 343b
On Wed, 13 Sep 2017 16:41:38 -0700, tonyc wrote: Show quoted text
> > + } > + require '../../t/test.pl'; > +} > + > +use strict; > +use warnings; > + > +use B (); > > Why use test.pl here instead of Test::More - the code doesn't use any > of the special functions (like runperl()) that test.pl provides. > > Tony
Quite Right. Attaching a new patch 4
Subject: 0004-ext-B-t-strict.t-Assure-B.pm-and-O.pm-pass-strict-an.patch
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Todd Rinaldo <toddr@cpan.org> Date: Wed, 13 Sep 2017 14:35:39 -0500 Subject: [PATCH 4/4] ext/B/t/strict.t: Assure B.pm and O.pm pass strict and warnings checks. We need to test these in unit tests since they do not load these modules to prevent pollution of the stash in compilers. --- MANIFEST | 1 + ext/B/t/strict.t | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 ext/B/t/strict.t diff --git a/MANIFEST b/MANIFEST index ad24a2d28b..5bcafdc8c2 100644 --- a/MANIFEST +++ b/MANIFEST @@ -3915,6 +3915,7 @@ ext/B/t/optree_varinit.t my,our,local var init optimization ext/B/t/OptreeCheck.pm optree comparison tool ext/B/t/pragma.t See if user pragmas work. ext/B/t/showlex.t See if B::ShowLex works +ext/B/t/strict.t See if B works with strict and warnings. ext/B/t/sv_stash.t See if SvSTASH() works ext/B/t/terse.t See if B::Terse works ext/B/t/walkoptree.t See if B::walkoptree (and friends) work diff --git a/ext/B/t/strict.t b/ext/B/t/strict.t new file mode 100644 index 0000000000..4d1b84aa20 --- /dev/null +++ b/ext/B/t/strict.t @@ -0,0 +1,30 @@ +#!./perl -w + +use strict; +use warnings; + +use Config; +use Test::More; + +BEGIN { + if ( ( $Config{'extensions'} !~ /\sB\s/ ) ) { + plan skip_all => "Perl was not compiled with B"; + exit 0; + } +} + +use strict; +use warnings; + +use B (); +use O (); + +foreach my $module (qw/B O/) { + my $path = $INC{ $module . '.pm' }; + my $check = "$^X -cw -Mstrict $path 2>&1"; + my $got = `$check`; + is( $got, "$path syntax OK\n", "$module.pm compiles without errors" ) + or diag($got); +} + +done_testing();
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 494b
On Wed, 13 Sep 2017 18:11:51 -0700, TODDR wrote: Show quoted text
> On Wed, 13 Sep 2017 16:41:38 -0700, tonyc wrote:
> > > > + } > > + require '../../t/test.pl'; > > +} > > + > > +use strict; > > +use warnings; > > + > > +use B (); > > > > Why use test.pl here instead of Test::More - the code doesn't use any > > of the special functions (like runperl()) that test.pl provides. > > > > Tony
> > > Quite Right. Attaching a new patch 4
I think we're waiting on Dave to review and merge. Bump? Todd
RT-Send-CC: perl5-porters [...] perl.org, davem [...] iabyn.com, perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 524b
bumping the case to check what is its status and what are the blockers for this case. thanks On Wed, 13 Sep 2017 18:11:51 -0700, TODDR wrote: Show quoted text
> On Wed, 13 Sep 2017 16:41:38 -0700, tonyc wrote:
> > > > + } > > + require '../../t/test.pl'; > > +} > > + > > +use strict; > > +use warnings; > > + > > +use B (); > > > > Why use test.pl here instead of Test::More - the code doesn't use any > > of the special functions (like runperl()) that test.pl provides. > > > > Tony
> > > Quite Right. Attaching a new patch 4
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #130087] Reduce module dependencies of B and O to simplify compiler second guessing.
Date: Sat, 4 Nov 2017 22:17:37 +0000
To: Atoomic via RT <perlbug-followup [...] perl.org>
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 911b
On Wed, Oct 04, 2017 at 08:51:11AM -0700, Atoomic via RT wrote: Show quoted text
> bumping the case to check what is its status and what are the blockers for this case. > thanks > > On Wed, 13 Sep 2017 18:11:51 -0700, TODDR wrote:
> > On Wed, 13 Sep 2017 16:41:38 -0700, tonyc wrote:
> > > > > > + } > > > + require '../../t/test.pl'; > > > +} > > > + > > > +use strict; > > > +use warnings; > > > + > > > +use B (); > > > > > > Why use test.pl here instead of Test::More - the code doesn't use any > > > of the special functions (like runperl()) that test.pl provides. > > > > > > Tony
> > > > > > Quite Right. Attaching a new patch 4
I'm happy with the patches. -- "Strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses, not from some farcical aquatic ceremony." -- Dennis, "Monty Python and the Holy Grail"
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 151b
On Sat, 04 Nov 2017 15:17:55 -0700, davem wrote: Show quoted text
> > I'm happy with the patches.
Merged to blead with commit 645a04a443d51be04bd2b42bdeb6f4d609eba522


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org