search

Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.95k stars 554 forks source link

Semi-Semicolons test suite is broken since perl 5.13.7 #11212

Closed p5pRT closed 13 years ago

p5pRT commented 13 years ago

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

Searchable as RT87064$

p5pRT commented 13 years ago

From @eserte

This is a bug report for perl from slaven@​rezic.de\, generated with the help of perlbug 1.39 running under perl 5.13.11.

The problem is described in https://rt.cpan.org/Ticket/Display.html?id=66876

The author (MSCHWERN) suggests that the breakage could be caused by bleadperl and should be reported here.

Regards\,   Slaven

Flags​:   category=core   severity=low

Site configuration information for perl 5.13.11​:

Configured by cpansand at Wed Mar 23 09​:22​:59 CET 2011.

Summary of my perl5 (revision 5 version 13 subversion 11) configuration​:   Commit id​: 167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc   Platform​:   osname=freebsd\, osvers=8.0-release\, archname=i386-freebsd   uname='freebsd biokovo.herceg.de 8.0-release freebsd 8.0-release #0​: sat nov 21 15​:48​:17 utc 2009 root@​almeida.cse.buffalo.edu​:usrobjusrsrcsysgeneric i386 '   config_args='-ds -e -Uversiononly -Dinstallusrbinperl=n -Dusedevel -Dprefix=/home/cpansand/var/ctps/2pre5140/install/perl-167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=undef\, usemultiplicity=undef   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=undef\, use64bitall=undef\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'\,   optimize='-O'\,   cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.2.1 20070719 [FreeBSD]'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12   ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=4\, prototype=define   Linker and Libraries​:   ld='cc'\, ldflags ='-Wl\,-E -fstack-protector -L/usr/local/lib'   libpth=/usr/lib /usr/local/lib   libs=-lgdbm -lm -lcrypt -lutil -lc   perllibs=-lm -lcrypt -lutil -lc   libc=\, so=so\, useshrplib=false\, libperl=libperl.a   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags='-DPIC -fPIC'\, lddlflags='-shared -L/usr/local/lib -fstack-protector'

Locally applied patches​:  

@​INC for perl 5.13.11​:   /home/cpansand/var/ctps/2pre5140/install/perl-167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc/lib/site_perl/5.13.11/i386-freebsd   /home/cpansand/var/ctps/2pre5140/install/perl-167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc/lib/site_perl/5.13.11   /home/cpansand/var/ctps/2pre5140/install/perl-167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc/lib/5.13.11/i386-freebsd   /home/cpansand/var/ctps/2pre5140/install/perl-167630b6ab7e291cbd4f89943a3aec8d6a1ecbfc/lib/5.13.11   .

Environment for perl 5.13.11​:   HOME=/home/e/eserte   LANG (unset)   LANGUAGE (unset)   LC_ALL=de_DE.ISO8859-1   LC_CTYPE=de_DE.ISO8859-1   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/local/bin​:/usr/bin​:/bin​:/usr/gnu/bin​:/usr/TeX/bin​:/usr/local/sbin​:/usr/sbin​:/sbin​:/usr/local/pilot/bin​:/home/e/eserte/bin/FreeBSD​:/home/e/eserte/bin/sh​:/home/e/eserte/bin​:/usr/X11R6/bin​:/usr/X11/bin​:/usr/X386/bin​:/usr/games​:/home/e/eserte/devel   PERLDOC=-MPod​::Perldoc​::ToTextOverstrike   PERL_BADLANG (unset)   PERL_HTML_DISPLAY_CLASS=HTML​::Display​::Mozilla   SHELL=/bin/tcsh

p5pRT commented 13 years ago

From @cpansprout

On Sat Mar 26 05​:33​:43 2011\, slaven@​rezic.de wrote​:

----------------------------------------------------------------- The problem is described in https://rt.cpan.org/Ticket/Display.html?id=66876

The author (MSCHWERN) suggests that the breakage could be caused by bleadperl and should be reported here.

This was caused by the following commit​:

commit f07ec6dd59215a56bc1159449a9631be7a02a94d Author​: Zefram \zefram@​fysh\.org Date​: Wed Oct 13 19​:05​:19 2010 +0100

  remove filter inheritance option from lex_start  
  The only uses of lex_start that had the new_filter parameter false\,   to make the new lexer context share source filters with the previous   lexer context\, were uses with rsfp null\, which therefore never invoked   source filters. Inheriting source filters from a logically unrelated   file seems like a silly idea anyway.

Whatā€™s happening is that Semi​::Semicolonsā€™ test suite is doing

BEGIN{ use_ok('Semi​::Semicolons'); }

which enables the source filter for the rest of the test script.

Test​::More implements use_ok with a ā€˜useā€™ statement inside a string eval\, so that line is a bit like​:

BEGIN{ eval "use Semi​::Semicolons" }

This now fails to activate the source filter because the eval and the test script are no longer able to share the same source filter space. When that commit messages says that ā€˜rsfp [was] null\, which therefore never invoked source filtersā€™\, it fails to take into account that\, even though source filters do not apply to string evals\, they can still allow filters to be propagated *back* to the real file currently being compiled.

To resolve this\, either we can just revert that patch (which would be an API change\, since that commit came just a few commits before lex_startā€™s being made public)\, or we can revert it\, rename lex_start to something else\, and make a lex_start macro with the current API.

p5pRT commented 13 years ago

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

p5pRT commented 13 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

                            it fails to take into account that\, even

though source filters do not apply to string evals\, they can still allow filters to be propagated *back* to the real file currently being compiled.

Eww. Let's not restore that behaviour. Change whatever was relying on it.

-zefram

p5pRT commented 13 years ago

From @rgarcia

On 28 March 2011 12​:39\, Zefram \zefram@​fysh\.org wrote​:

Father Chrysostomos via RT wrote​:

Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  Ā  it fails to take into account that\, even though source filters do not apply to string evals\, they can still allow filters to be propagated *back* to the real file currently being compiled.

Eww. Ā Let's not restore that behaviour. Ā Change whatever was relying on it.

I would second that; anything that makes source filters a bit less intrusive is good. However I suppose that this needs an entry in perldelta / incompatible changes.

p5pRT commented 13 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

Test​::More implements use_ok with a ???use??? statement inside a string eval\, so that line is a bit like​:

It occurred to me that not only does this now run into trouble with source filters but it'll also\, regardless of the recent source filter change\, run into trouble with anything that interacts with the compilation context. Any import method that sets up a lexically-scoped effect will influence the lexical scope of the string eval\, not the lexical scope of the BEGIN{use_ok}. Likewise\, any source injection into the parsing context\, triggered from the import method\, will affect the eval rather than the main code. Example​:

$ perl -le 'use Test​::More tests => 2; BEGIN { use_ok("strict"\, "vars"); } $z; ok 1;' 1..2 ok 1 - use strict; ok 2

That ought to fail at compile time\, like this​:

$ perl -le 'use Test​::More tests => 2; use strict "vars"; BEGIN { ok 1; } $z; ok 1;' 1..2 ok 1 Global symbol "$z" requires explicit package name at -e line 1. Execution of -e aborted due to compilation errors. # Looks like you planned 2 tests but ran 1. # Looks like your test exited with 255 just after 1.

So I reckon use_ok is seriously broken\, and the core change has just made this brokenness show up in a new situation. use_ok needs to do the require and import itself\, without any string eval. The core of it is something like​:

  sub my_use($;@​) {   my $module = $_[0];   (my $filename = $module.".pm") =~ s!​::!/!g;   require($filename);   goto &{$module->can("import")};   }

-zefram

p5pRT commented 13 years ago

From @iabyn

On Tue\, Mar 29\, 2011 at 12​:16​:33PM +0100\, Zefram wrote​:

So I reckon use_ok is seriously broken\, and the core change has just made this brokenness show up in a new situation. use_ok needs to do the require and import itself\, without any string eval. The core of it is something like​:

sub my\_use\($;@​\) \{
    my $module = $\_\[0\];
    \(my $filename = $module\."\.pm"\) =~ s\!​::\!/\!g;
    require\($filename\);
    goto &\{$module\->can\("import"\)\};
\}

But of course\, if does the goto (to get the correct caller lexical scope)\, then it's too late to print 'not ok' if the import fails (e.g. if it dies).

So I suspect that use_ok() is fundamentally unsuitable (and not fixable) for modules that affect the callers lexical scope.

In which case\, it's probably best to just document this limitation in Test​::More\, and suggest to Semi-Semicolons's maintainer to avoid use_ok().

If this is acceptable\, then this ticket ceases to be a 5.14 blocker.

-- Lear​: Dost thou call me fool\, boy? Fool​: All thy other titles thou hast given away; that thou wast born with.

p5pRT commented 13 years ago

From zefram@fysh.org

Dave Mitchell wrote​:

But of course\, if does the goto (to get the correct caller lexical scope)\, then it's too late to print 'not ok' if the import fails (e.g. if it dies).

True\, but it's not a disaster if we change use_ok() to work this way. It can print "ok" or "not ok" based on whether the require() succeeds\, and then chain the import. If import dies then the test script dies\, which should adequately signal failure. The only real losers are test scripts that relied on being able to continue with other tests after use_ok()\, but that must be a rare usage​: usually the later tests depend on the effects of the import and so will fail anyway.

So I suspect that use_ok() is fundamentally unsuitable (and not fixable) for modules that affect the callers lexical scope.

I think what I outlined above is a better tradeoff than leaving use_ok() unsuitable for anything with lexical effect. But probably not a change to make before 5.14.

In which case\, it's probably best to just document this limitation in Test​::More\, and suggest to Semi-Semicolons's maintainer to avoid use_ok().

I think this is the best approach for 5.14.

-zefram

p5pRT commented 13 years ago

From @cpansprout

On Wed Mar 30 07​:32​:21 2011\, zefram@​fysh.org wrote​:

Dave Mitchell wrote​:

But of course\, if does the goto (to get the correct caller lexical scope)\, then it's too late to print 'not ok' if the import fails (e.g. if it dies).

True\, but it's not a disaster if we change use_ok() to work this way. It can print "ok" or "not ok" based on whether the require() succeeds\, and then chain the import. If import dies then the test script dies\, which should adequately signal failure. The only real losers are test scripts that relied on being able to continue with other tests after use_ok()\, but that must be a rare usage​: usually the later tests depend on the effects of the import and so will fail anyway.

So I suspect that use_ok() is fundamentally unsuitable (and not fixable) for modules that affect the callers lexical scope.

I think what I outlined above is a better tradeoff than leaving use_ok() unsuitable for anything with lexical effect. But probably not a change to make before 5.14.

This is actually more tricky than you suggest\, as Test​::More currently supports use_ok("Exporter 5.57") syntax.

In which case\, it's probably best to just document this limitation in Test​::More\, and suggest to Semi-Semicolons's maintainer to avoid use_ok().

I think this is the best approach for 5.14.

Do you realise there are twenty distributions affected?

Acme​::Base64 Acme​::ComeFrom Acme​::Hyperindex Acme​::Lingua​::NIGERIAN Acme​::Pythonic Call​::Immediate Data​::Dumper​::Simple Filter​::Arguments Filter​::BoxString Filter​::CommaEquals Filter​::Indent​::HereDoc Filter​::Object​::Simple Filter​::Template For​::Else Lingua​::tlhInganHol​::yIghun Net​::IP​::Match Perl6​::Attributes Semi​::Semicolons Sub​::Signatures Test​::Standalone

I would suggest that we re-instate the filter-sharing capability (see the attached patch) as a stopgap measure. We could always revert it after 5.14.

(Slaven\, Iā€™m forwarding this to you\, as these distributions are on your list.)

p5pRT commented 13 years ago

From @cpansprout

From 2f1624fb1b763c9899594d1671941477b7df2091 Mon Sep 17 00​:00​:00 2001 From​: Father Chrysostomos \sprout@​cpan\.org Date​: Tue\, 29 Mar 2011 08​:33​:30 -0700

[perl #87064] eval no longer shares filters

Before this commit​:

commit f07ec6dd59215a56bc1159449a9631be7a02a94d Author​: Zefram \zefram@​fysh\.org Date​: Wed Oct 13 19​:05​:19 2010 +0100

  remove filter inheritance option from lex_start

  The only uses of lex_start that had the new_filter parameter false\,   to make the new lexer context share source filters with the previous   lexer context\, were uses with rsfp null\, which therefore never invoked   source filters. Inheriting source filters from a logically unrelated   file seems like a silly idea anyway.

string evals could inherit the same source filter space as the cur- rently compiling code. Despite what the quoted commit message says\, sharing source filters allows filters to be inherited in both direc- tions​: A source filter created when the eval is being compiled also applies to the file with which it is sharing its space.

There are at least 20 CPAN distributions relying on this behaviour. So this commit restores the source-filter-sharing capability. It does not change the current API or make public the API for sharing source filters\, as this is supposed to be a minimial change because of the code freeze.

Inline Patch ```diff diff --git a/MANIFEST b/MANIFEST index 1e2189e..0fea93d 100644 --- a/MANIFEST +++ b/MANIFEST @@ -3633,6 +3633,7 @@ ext/XS-APItest/t/cleanup.t test stack behaviour on unwinding ext/XS-APItest/t/cophh.t test COPHH API ext/XS-APItest/t/copyhints.t test hv_copy_hints_hv() API ext/XS-APItest/t/customop.t XS::APItest: tests for custom ops +ext/XS-APItest/t/eval-filter.t Simple source filter/eval test ext/XS-APItest/t/exception.t XS::APItest extension ext/XS-APItest/t/grok.t XS::APItest: tests for grok* functions ext/XS-APItest/t/hash.t XS::APItest: tests for hash related APIs diff --git a/ext/XS-APItest/APItest.pm b/ext/XS-APItest/APItest.pm index b95af26..5ef9ea2 100644 --- a/ext/XS-APItest/APItest.pm +++ b/ext/XS-APItest/APItest.pm @@ -50,7 +50,7 @@ sub import { } } -our $VERSION = '0.27'; +our $VERSION = '0.28'; use vars '$WARNINGS_ON_BOOTSTRAP'; use vars map "\$${_}_called_PP", qw(BEGIN UNITCHECK CHECK INIT END); @@ -209,6 +209,11 @@ correctly by C. Output is sent to STDOUT. +=item B + +Installs a source filter that substitutes "e" for "o" (witheut regard fer +what it might be medifying). + =item B, B, B These exercise the C calls of the same names. Everything after the flags diff --git a/ext/XS-APItest/APItest.xs b/ext/XS-APItest/APItest.xs index 0ce4d51..4fa4e1e 100644 --- a/ext/XS-APItest/APItest.xs +++ b/ext/XS-APItest/APItest.xs @@ -996,6 +996,26 @@ peep_xop(pTHX_ OP *o, OP *oldop) av_push(MY_CXT.xop_record, newSVpvf("oldop:%"UVxf, PTR2UV(oldop))); } +static I32 +filter_call(pTHX_ int idx, SV *buf_sv, int maxlen) +{ + SV *my_sv = FILTER_DATA(idx); + char *p; + char *end; + int n = FILTER_READ(idx + 1, buf_sv, maxlen); + + if (n<=0) return n; + + p = SvPV_force_nolen(buf_sv); + end = p + SvCUR(buf_sv); + while (p < end) { + if (*p == 'o') *p = 'e'; + p++; + } + return SvCUR(buf_sv); +} + + XS(XS_XS__APItest__XSUB_XS_VERSION_undef); XS(XS_XS__APItest__XSUB_XS_VERSION_empty); XS(XS_XS__APItest__XSUB_XS_APIVERSION_invalid); @@ -2750,6 +2770,11 @@ CODE: PERL_UNUSED_VAR(items); croak("postinc called as a function"); +void +filter() +CODE: + filter_add(filter_call, NULL); + BOOT: { CV *asscv = get_cv("XS::APItest::postinc", 0); diff --git a/ext/XS-APItest/t/eval-filter.t b/ext/XS-APItest/t/eval-filter.t new file mode 100644 index 0000000..8d370e5 --- /dev/null +++ b/ext/XS-APItest/t/eval-filter.t @@ -0,0 +1,10 @@ +#!perl -w +use strict; + +use Test::More tests => 1; +use XS::APItest; + +BEGIN { eval "BEGIN{ filter() }" } + +is "foo", "fee", "evals share filters with the currently compiling scope"; +# See [perl #87064]. diff --git a/op.c b/op.c index 2e15a8d..e917d43 100644 --- a/op.c +++ b/op.c @@ -4453,7 +4453,7 @@ Perl_vload_module(pTHX_ U32 flags, SV *name, SV *ver, va_list *args) ENTER; SAVEVPTR(PL_curcop); - lex_start(NULL, NULL, 0); + lex_start(NULL, NULL, LEX_START_SAME_FILTER); utilize(!(flags & PERL_LOADMOD_DENY), start_subparse(FALSE, 0), veop, modname, imop); LEAVE; diff --git a/parser.h b/parser.h index e4a06dc..17ced8f 100644 --- a/parser.h +++ b/parser.h @@ -112,6 +112,9 @@ typedef struct yy_parser { /* flags for lexer API */ #define LEX_STUFF_UTF8 0x00000001 #define LEX_KEEP_PREVIOUS 0x00000002 +#ifdef PERL_CORE +# define LEX_START_SAME_FILTER 0x00000001 +#endif /* flags for parser API */ #define PARSE_OPTIONAL 0x00000001 diff --git a/pp_ctl.c b/pp_ctl.c index 44cf3c1..aabbcd3 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -3090,7 +3090,7 @@ Perl_sv_compile_2op_is_broken(pTHX_ SV *sv, OP **startop, const char *code, PERL_ARGS_ASSERT_SV_COMPILE_2OP_IS_BROKEN; ENTER_with_name("eval"); - lex_start(sv, NULL, 0); + lex_start(sv, NULL, LEX_START_SAME_FILTER); SAVETMPS; /* switch to eval mode */ @@ -3957,7 +3957,7 @@ PP(pp_entereval) TAINT_PROPER("eval"); ENTER_with_name("eval"); - lex_start(sv, NULL, 0); + lex_start(sv, NULL, LEX_START_SAME_FILTER); SAVETMPS; /* switch to eval mode */ diff --git a/toke.c b/toke.c index 9642ad3..f5f1f8a 100644 --- a/toke.c +++ b/toke.c @@ -667,11 +667,15 @@ code in I comes first and must consist of complete lines of input, and I supplies the remainder of the source. The I parameter is reserved for future use, and must always -be zero. +be zero, except for one flag that is currently reserved for perl's internal +use. =cut */ +/* LEX_START_SAME_FILTER indicates that this is not a new file, so it + can share filters with the current parser. */ + void Perl_lex_start(pTHX_ SV *line, PerlIO *rsfp, U32 flags) { @@ -679,7 +683,7 @@ Perl_lex_start(pTHX_ SV *line, PerlIO *rsfp, U32 flags) const char *s = NULL; STRLEN len; yy_parser *parser, *oparser; - if (flags) + if (flags && flags != LEX_START_SAME_FILTER) Perl_croak(aTHX_ "Lexing code internal error (%s)", "lex_start"); /* create and initialise a parser */ @@ -708,7 +712,10 @@ Perl_lex_start(pTHX_ SV *line, PerlIO *rsfp, U32 flags) parser->lex_state = LEX_NORMAL; parser->expect = XSTATE; parser->rsfp = rsfp; - parser->rsfp_filters = newAV(); + parser->rsfp_filters = + !(flags & LEX_START_SAME_FILTER) || !oparser + ? newAV() + : MUTABLE_AV(SvREFCNT_inc(oparser->rsfp_filters)); Newx(parser->lex_brackstack, 120, char); Newx(parser->lex_casestack, 12, char); ```
p5pRT commented 13 years ago

From @cpansprout

On Sun Apr 03 13​:20​:54 2011\, sprout wrote​:

(Slaven\, Iā€™m forwarding this to you\, as these distributions are on your list.)

(Except now I realise that you were the one who opened this ticket to begin with\, so you would have received it anyway.)

p5pRT commented 13 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

This is actually more tricky than you suggest\, as Test​::More currently supports use_ok("Exporter 5.57") syntax.

That's only a minor complication. Didn't seem worth including it in the sketch of how use_ok should operate.

Do you realise there are twenty distributions affected?

No\, I didn't. That changes the tradeoff calculation a bit. Sounds better to go right to the use_ok that I proposed.

I would suggest that we re-instate the filter-sharing capability (see the attached patch) as a stopgap measure. We could always revert it after 5.14.

Mmhmm. I'd be happier with fixing use_ok\, but I can live with this.

-zefram

p5pRT commented 13 years ago

From @cpansprout

On Sun Apr 03 13​:26​:50 2011\, zefram@​fysh.org wrote​:

Father Chrysostomos via RT wrote​:

This is actually more tricky than you suggest\, as Test​::More currently supports use_ok("Exporter 5.57") syntax.

That's only a minor complication. Didn't seem worth including it in the sketch of how use_ok should operate.

Do you realise there are twenty distributions affected?

No\, I didn't. That changes the tradeoff calculation a bit. Sounds better to go right to the use_ok that I proposed.

I would suggest that we re-instate the filter-sharing capability (see the attached patch) as a stopgap measure. We could always revert it after 5.14.

Mmhmm. I'd be happier with fixing use_ok\,

So would I\, if it were easier.

but I can live with this.

OK. Iā€™ve just applied it as 27fcb6e. Iā€™ll see if I can write a Test​::More patch\, maybe in the next week or two.

p5pRT commented 13 years ago

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

p5pRT commented 13 years ago

From @schwern

On 2011.4.4 9​:22 AM\, Father Chrysostomos via RT wrote​:

OK. Iā€™ve just applied it as 27fcb6e. Iā€™ll see if I can write a Test​::More patch\, maybe in the next week or two.

This might be a good excuse to assimilate ok.pm. http​://search.cpan.org/~audreyt/Test-use-ok-0.02/lib/ok.pm

-- 164. There is no such thing as a were-virgin.   -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army   http​://skippyslist.com/list/