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

%INC caching failure-case problem #7534

Closed
p5pRT opened this issue Oct 10, 2004 · 14 comments
Closed

%INC caching failure-case problem #7534

p5pRT opened this issue Oct 10, 2004 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 10, 2004

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

Searchable as RT31924$

@p5pRT
Copy link
Author

p5pRT commented Oct 10, 2004

From @nwc10

Created by @nwc10

This seems to be a problem only in blead​:

$ echo die >fail.pm
$ ./perl -Ilib -e 'eval "use fail q{​:std};"; $INC{"fail.pm"} = "fail.pm"'
Modification of non-creatable hash value attempted, subscript "fail.pm" at -e line 1.

I think it's something to do with the code in blead which cached failure
cases for require, code which isn't in maint. (And I'm now more inclined to
leave it that way)

The bug seems to need something aggressive such as a die inside the other
module to trigger the failure case - simply not returning true from the
module won't trigger it.

The code example is real world - it's what you get from lib/if.t if
lib/open.pm fails to compile.

Nicholas Clark

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl v5.9.2:

Configured by nick at Sun Oct 10 15:49:37 BST 2004.

Summary of my perl5 (revision 5 version 9 subversion 2 patch 22511) configuration:
  Platform:
    osname=freebsd, osvers=5.2.1-release-p9, archname=i386-freebsd
    uname='freebsd saigo.etla.org 5.2.1-release-p9 freebsd 5.2.1-release-p9 #3: sat jul 3 19:29:18 bst 2004 root@saigo.etla.org:usrobjusrsrcsyssaigo i386 '
    config_args='-Dusedevel=y -Dcc=ccache gcc -Dld=gcc -Ubincompat5005 -Uinstallusrbinperl -Dcf_email=nick@ccl4.org -Dperladmin=nick@ccl4.org -Dinc_version_list=  -Dinc_version_list_init=0 -Doptimize=-O2 -Accflags=-std=c89 -ansi -pedantic -DPERL_GCC_PEDANTIC -Dusethreads=n -Uuse64bitint -Dnoextensions=Encode -Uusemymalloc -Dprefix=~/Sandpit/snap5.9.x-23354 -Dinstallman1dir=none -Dinstallman3dir=none -de'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef 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='ccache gcc', ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -std=c89 -ansi -pedantic -DPERL_GCC_PEDANTIC -fno-strict-aliasing -pipe -I/usr/local/include',
    optimize='-O2',
    cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -std=c89 -ansi -pedantic -DPERL_GCC_PEDANTIC -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='3.3.3 [FreeBSD] 20031106', 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='gcc', ldflags ='-Wl,-E  -L/usr/local/lib'
    libpth=/usr/lib /usr/local/lib
    libs=-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'

Locally applied patches:
    DEVEL22511


@INC for perl v5.9.2:
    lib
    /home/nick/Sandpit/snap5.9.x-23354/lib/perl5/5.9.2/i386-freebsd
    /home/nick/Sandpit/snap5.9.x-23354/lib/perl5/5.9.2
    /home/nick/Sandpit/snap5.9.x-23354/lib/perl5/site_perl/5.9.2/i386-freebsd
    /home/nick/Sandpit/snap5.9.x-23354/lib/perl5/site_perl/5.9.2
    /home/nick/Sandpit/snap5.9.x-23354/lib/perl5/site_perl
    .


Environment for perl v5.9.2:
    HOME=/home/nick
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/nick/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/home/nick/bin:/usr/local/sbin:/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2004

From rick@bort.ca

On Sun, Oct 10, 2004 at 03​:38​:35PM -0000, Nicholas Clark wrote​:

$ echo die >fail.pm
$ ./perl -Ilib -e 'eval "use fail q{​:std};"; $INC{"fail.pm"} = "fail.pm"'
Modification of non-creatable hash value attempted, subscript "fail.pm" at -e line 1.

I think it's something to do with the code in blead which cached failure
cases for require, code which isn't in maint. (And I'm now more inclined to
leave it that way)

This is because as it is now, failed compiles are cached as PL_sv_undef
which pp_helem is specifically coded to reject; I'm not sure why.

The bug seems to need something aggressive such as a die inside the other
module to trigger the failure case - simply not returning true from the
module won't trigger it.

Right, because those cases are not cached as failures.

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

  keys %INC;

should only return the names of files successfully loaded.

There was no API that I could find to test if a key was in %INC and was
a placeholder so I added one (hv_fetch_flags). If there was another way
or if it would have been better to make hv_fetch_common public then feel
free to point me in the right direction and I'll change it. If
hv_fetch_flags is ok then I suppose hv_fetch should be changed to call
it.

I didn't update the require subroutine in perlfunc. I really believe
that it exposes too much implementation for no reason and should just be
deleted.

--
Rick Delaney
rick@​bort.ca

Inline Patch
--- perl-current/embed.fnc	Sun Oct 10 15:41:03 2004
+++ perl-dev/embed.fnc	Tue Oct 12 23:07:36 2004
@@ -271,6 +271,7 @@
 Apd	|bool	|hv_exists	|HV* tb|const char* key|I32 klen
 Apd	|bool	|hv_exists_ent	|HV* tb|SV* key|U32 hash
 Apd	|SV**	|hv_fetch	|HV* tb|const char* key|I32 klen|I32 lval
+ApMd	|SV**	|hv_fetch_flags	|HV* tb|const char* key|I32 klen|I32 lval|I32 flags
 Apd	|HE*	|hv_fetch_ent	|HV* tb|SV* key|I32 lval|U32 hash
 Ap	|void	|hv_free_ent	|HV* hv|HE* entry
 Apd	|I32	|hv_iterinit	|HV* tb
--- perl-current/hv.c	Sat Jul 31 12:44:58 2004
+++ perl-dev/hv.c	Wed Oct 13 00:29:21 2004
@@ -186,6 +186,7 @@
 #define HV_FETCH_ISEXISTS  0x02
 #define HV_FETCH_LVALUE    0x04
 #define HV_FETCH_JUST_SV   0x08
+#define HV_FETCH_PLACEHOLDER   0x10
 
 /*
 =for apidoc hv_store
@@ -337,6 +338,46 @@
 }
 
 /*
+=for apidoc hv_fetch_flags
+
+Returns the SV which corresponds to the specified key in the hash.
+See C<hv_fetch>.
+The C<flags> value will normally be zero; if HV_FETCH_WANTPLACEHOLDERS is
+set then placeholders keys (for restricted hashes) will be returned in addition
+to normal keys. By default placeholders are automatically skipped over.
+Currently a placeholder is implemented with a value that is
+C<&Perl_sv_placeholder>. Note that the implementation of placeholders and
+restricted hashes may change.
+
+=cut
+*/
+
+SV**
+Perl_hv_fetch_flags(pTHX_ HV *hv, const char *key, I32 klen_i32, I32 lval, 
+		    I32 flags)
+{
+    HE *hek;
+    STRLEN klen;
+    int common_flags;
+
+    if (klen_i32 < 0) {
+	klen = -klen_i32;
+	common_flags = HVhek_UTF8;
+    } else {
+	klen = klen_i32;
+	common_flags = 0;
+    }
+    hek = hv_fetch_common (hv, NULL, key, klen, common_flags,
+			   ((flags & HV_FETCH_WANTPLACEHOLDERS) 
+				? HV_FETCH_PLACEHOLDER 
+				: 0)
+			    | HV_FETCH_JUST_SV 
+			    | (lval ? HV_FETCH_LVALUE : 0),
+			   Nullsv, 0);
+    return hek ? &HeVAL(hek) : NULL;
+}
+
+/*
 =for apidoc hv_exists_ent
 
 Returns a boolean indicating whether the specified hash key exists. C<hash>
@@ -693,7 +734,9 @@
 		SvREFCNT_dec(HeVAL(entry));
 		HeVAL(entry) = val;
 	    }
-	} else if (HeVAL(entry) == &PL_sv_placeholder) {
+	} else if (HeVAL(entry) == &PL_sv_placeholder 
+		    && !(action & HV_FETCH_PLACEHOLDER)) 
+	{
 	    /* if we find a placeholder, we pretend we haven't found
 	       anything */
 	    break;
--- perl-current/hv.h	Mon Nov  3 03:23:31 2003
+++ perl-dev/hv.h	Tue Oct 12 22:59:33 2004
@@ -318,6 +318,9 @@
 /* Flags for hv_iternext_flags.  */
 #define HV_ITERNEXT_WANTPLACEHOLDERS	0x01	/* Don't skip placeholders.  */
 
+/* Flags for hv_fetch_flags.  */
+#define HV_FETCH_WANTPLACEHOLDERS	0x01	/* Don't skip placeholders.  */
+
 /* available as a function in hv.c */
 #define Perl_sharepvn(sv, len, hash) HEK_KEY(share_hek(sv, len, hash))
 #define sharepvn(sv, len, hash)	     Perl_sharepvn(sv, len, hash)
--- perl-current/pp_ctl.c	Sun Oct 10 15:41:04 2004
+++ perl-dev/pp_ctl.c	Tue Oct 12 23:26:28 2004
@@ -1469,7 +1469,7 @@
 		char* msg = SvPVx(ERRSV, n_a);
                SV *nsv = cx->blk_eval.old_namesv;
                (void)hv_store(GvHVn(PL_incgv), SvPVX(nsv), SvCUR(nsv),
-                               &PL_sv_undef, 0);
+                               &PL_sv_placeholder, 0);
 		DIE(aTHX_ "%sCompilation failed in require",
 		    *msg ? msg : "Unknown error\n");
 	    }
@@ -2949,7 +2949,7 @@
 	    char* msg = SvPVx(ERRSV, n_a);
            SV *nsv = cx->blk_eval.old_namesv;
            (void)hv_store(GvHVn(PL_incgv), SvPVX(nsv), SvCUR(nsv),
-                          &PL_sv_undef, 0);
+                          &PL_sv_placeholder, 0);
 	    DIE(aTHX_ "%sCompilation failed in require",
 		*msg ? msg : "Unknown error\n");
 	}
@@ -3091,8 +3091,10 @@
 	DIE(aTHX_ "Null filename used");
     TAINT_PROPER("require");
     if (PL_op->op_type == OP_REQUIRE &&
-       (svp = hv_fetch(GvHVn(PL_incgv), name, len, 0))) {
-       if (*svp != &PL_sv_undef)
+       (svp = hv_fetch_flags(GvHVn(PL_incgv), name, len, 0,
+		       HV_FETCH_WANTPLACEHOLDERS)))
+    {
+       if (*svp != &PL_sv_placeholder)
            RETPUSHYES;
        else
            DIE(aTHX_ "Compilation failed in require");
--- perl-current/t/comp/require.t	Wed Aug  4 02:42:46 2004
+++ perl-dev/t/comp/require.t	Wed Oct 13 00:34:32 2004
@@ -11,8 +11,9 @@
 
 my $Is_EBCDIC = (ord('A') == 193) ? 1 : 0;
 my $Is_UTF8   = (${^OPEN} || "") =~ /:utf8/;
-my $total_tests = 44;
-if ($Is_EBCDIC || $Is_UTF8) { $total_tests = 41; }
+my $total_tests = 43;
+my $ebcdic_utf8_skips = 3;
+if ($Is_EBCDIC || $Is_UTF8) { $total_tests -= $ebcdic_utf8_skips; }
 print "1..$total_tests\n";
 
 sub do_require {
@@ -122,8 +123,6 @@
     print "ok ",$i++,"\n";
     print "not " unless -e $flag_file xor $expected_compile;
     print "ok ",$i++,"\n";
-    print "not " unless exists $INC{'bleah.pm'};
-    print "ok ",$i++,"\n";
 }
 
 # compile-time failure in require
@@ -133,9 +132,6 @@
 print "# $@\nnot " unless $@ =~ /(syntax|parse) error/mi;
 print "ok ",$i++,"\n";
 
-# previous failure cached in %INC
-print "not " unless exists $INC{'bleah.pm'};
-print "ok ",$i++,"\n";
 write_file($flag_file, 1);
 write_file('bleah.pm', "unlink '$flag_file'; 1");
 print "# $@\nnot " if eval { require 'bleah.pm' };
@@ -144,12 +140,18 @@
 print "ok ",$i++,"\n";
 print "not " unless -e $flag_file;
 print "ok ",$i++,"\n";
-print "not " unless exists $INC{'bleah.pm'};
+# [perl #31924]
+eval { $INC{'bleah.pm'} = 'bleah.pm' };
+print "# $@\nnot " if $@;
+print "ok ",$i++,"\n";
+print "not " unless $INC{'bleah.pm'} eq 'bleah.pm';
 print "ok ",$i++,"\n";
 
 # successful require
 do_require "1";
 print "# $@\nnot " if $@;
+print "ok ",$i++,"\n";
+print "not " unless $INC{'bleah.pm'} eq 'bleah.pm';
 print "ok ",$i++,"\n";
 
 # do FILE shouldn't see any outside lexicals

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2004

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

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2004

From @ysth

On Wed, Oct 13, 2004 at 12​:40​:18PM -0400, Rick Delaney <rick@​bort.ca> wrote​:

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

I don't think that's better. If we are going to cache failures, they
should be visible from perl too.

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2004

From rick@bort.ca

On Wed, Oct 13, 2004 at 10​:12​:50AM -0700, Yitzchak Scott-Thoennes wrote​:

On Wed, Oct 13, 2004 at 12​:40​:18PM -0400, Rick Delaney <rick@​bort.ca> wrote​:

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

I don't think that's better. If we are going to cache failures, they
should be visible from perl too.

Maybe, but I don't think they need be visible in %INC since they
haven't actually been INCluded.

--
Rick Delaney
rick@​bort.ca

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2004

From @rgs

Rick Delaney wrote​:

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

I don't think that's better. If we are going to cache failures, they
should be visible from perl too.

Maybe, but I don't think they need be visible in %INC since they
haven't actually been INCluded.

I agree with this on principle. However something bugs me in your patch :
the number of tests of t/op/require.t has been reduced by one. In
general if some behaviour has changed it's better to continue testing,
but for the new behaviour.

--
Today the bards must drink and junket. Ireland expects that every man
this day will do his duty.
  -- Ulysses

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2004

From rick@bort.ca

On Thu, Oct 14, 2004 at 06​:58​:41PM +0200, Rafael Garcia-Suarez wrote​:

Rick Delaney wrote​:

Maybe, but I don't think they need be visible in %INC since they
haven't actually been INCluded.

I agree with this on principle. However something bugs me in your patch :
the number of tests of t/op/require.t has been reduced by one. In
general if some behaviour has changed it's better to continue testing,
but for the new behaviour.

Yes, I actually removed 3 tests I think, all of which were testing that
$INC{$filename} existed for a cached failure. I'm pretty sure that when
I added those tests originally I mentioned that they were probably
unnecessary because they are testing the caching implementation and
there are other tests in there that verify the caching behaviour.

Anyway, since I was the one that originally added those tests I don't
feel bad about saying they were a mistake and removing them now. Plus
they're only in blead anyway.

--
Rick Delaney
rick@​bort.ca

@p5pRT
Copy link
Author

p5pRT commented Oct 14, 2004

From @ysth

On Thu, Oct 14, 2004 at 06​:58​:41PM +0200, Rafael Garcia-Suarez <rgarciasuarez@​mandrakesoft.com> wrote​:

Rick Delaney wrote​:

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

I don't think that's better. If we are going to cache failures, they
should be visible from perl too.

Maybe, but I don't think they need be visible in %INC since they
haven't actually been INCluded.

I agree with this on principle. However something bugs me in your patch :
the number of tests of t/op/require.t has been reduced by one. In
general if some behaviour has changed it's better to continue testing,
but for the new behaviour.

Then perhaps we need a %^NOINC? I really dislike the idea of having
hidden entries in %INC that result in require not attempting to load a
file but where the perl code doing the require can't detect that this
will happen.

@p5pRT
Copy link
Author

p5pRT commented Jan 21, 2005

From @rgs

Rick Delaney wrote​:

On Sun, Oct 10, 2004 at 03​:38​:35PM -0000, Nicholas Clark wrote​:

$ echo die >fail.pm
$ ./perl -Ilib -e 'eval "use fail q{​:std};"; $INC{"fail.pm"} = "fail.pm"'
Modification of non-creatable hash value attempted, subscript "fail.pm" at -e line 1.

I think it's something to do with the code in blead which cached failure
cases for require, code which isn't in maint. (And I'm now more inclined to
leave it that way)

This is because as it is now, failed compiles are cached as PL_sv_undef
which pp_helem is specifically coded to reject; I'm not sure why.

The bug seems to need something aggressive such as a die inside the other
module to trigger the failure case - simply not returning true from the
module won't trigger it.

Right, because those cases are not cached as failures.

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

Thanks, applied as #23843 to blead.

There was no API that I could find to test if a key was in %INC and was
a placeholder so I added one (hv_fetch_flags). If there was another way
or if it would have been better to make hv_fetch_common public then feel
free to point me in the right direction and I'll change it. If
hv_fetch_flags is ok then I suppose hv_fetch should be changed to call
it.

@p5pRT p5pRT closed this as completed Jan 21, 2005
@p5pRT
Copy link
Author

p5pRT commented Jan 21, 2005

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

@p5pRT
Copy link
Author

p5pRT commented Jan 23, 2005

From @ysth

On Fri, Jan 21, 2005 at 03​:02​:46PM +0100, Rafael Garcia-Suarez wrote​:

Rick Delaney wrote​:

On Sun, Oct 10, 2004 at 03​:38​:35PM -0000, Nicholas Clark wrote​:

$ echo die >fail.pm
$ ./perl -Ilib -e 'eval "use fail q{​:std};"; $INC{"fail.pm"} = "fail.pm"'
Modification of non-creatable hash value attempted, subscript "fail.pm" at -e line 1.

I think it's something to do with the code in blead which cached failure
cases for require, code which isn't in maint. (And I'm now more inclined to
leave it that way)

This is because as it is now, failed compiles are cached as PL_sv_undef
which pp_helem is specifically coded to reject; I'm not sure why.

The bug seems to need something aggressive such as a die inside the other
module to trigger the failure case - simply not returning true from the
module won't trigger it.

Right, because those cases are not cached as failures.

The following patch changes the caching placeholder to
PL_sv_placeholder which apart from being a better name is also better
because now

keys %INC;

should only return the names of files successfully loaded.

Thanks, applied as #23843 to blead.

There was no API that I could find to test if a key was in %INC and was
a placeholder so I added one (hv_fetch_flags). If there was another way
or if it would have been better to make hv_fetch_common public then feel
free to point me in the right direction and I'll change it. If
hv_fetch_flags is ok then I suppose hv_fetch should be changed to call
it.

My last try at arguing against this; hereafter I'll stay quiet.
The following​:

system q!echo 'sub foo{"bar"} die'>foo.pl!;
eval q!require "foo.pl"!;
print &foo();
delete $INC{"foo.pl"};
system q!echo 'sub foo{"baz"} 1'>foo.pl!;
require "foo.pl";
print &foo();

prints barbaz without this patch but​:
barCompilation failed in require at req.pl line 6.

with it. I really don't see a way to cache failures in %INC without
what I consider an ugly amount of backward incompatibility.

If this is really, truly, necessary, cache failures in %^NOINC instead
of %INC, and document it in perldelta for 5.10.

@p5pRT
Copy link
Author

p5pRT commented Jan 23, 2005

From rick@bort.ca

On Sun, Jan 23, 2005 at 12​:14​:30AM -0800, Yitzchak Scott-Thoennes wrote​:

My last try at arguing against this; hereafter I'll stay quiet.
The following​:

system q!echo 'sub foo{"bar"} die'>foo.pl!;
eval q!require "foo.pl"!;
print &foo();
delete $INC{"foo.pl"};
system q!echo 'sub foo{"baz"} 1'>foo.pl!;
require "foo.pl";
print &foo();

prints barbaz without this patch but​:
barCompilation failed in require at req.pl line 6.

with it. I really don't see a way to cache failures in %INC without
what I consider an ugly amount of backward incompatibility.

I think this is a reasonable objection. I'd be happy with it continuing
to cache fails as undef. The offending code for this bug is around line
1720 of pp_hot.c​:

  if (lval) {
  if (!svp || *svp == &PL_sv_undef) {
  SV* lv;
  SV* key2;
  if (!defer) {
  STRLEN n_a;
  DIE(aTHX_ PL_no_helem, SvPV(keysv, n_a));
  }

where defer is

  PL_op->op_private & OPpLVAL_DEFER;

and *svp is, of course, &PL_sv_undef.

I don't understand the purpose of the defer. Maybe it's possible to fix
that part and leave the rest alone. Maybe we could cache fails as "".
Or can an SV evaluate to undef without being PL_sv_undef?

If this is really, truly, necessary, cache failures in %^NOINC instead
of %INC, and document it in perldelta for 5.10.

I don't have a problem with this either, except that it won't fix the
code above.

--
Rick Delaney
rick@​bort.ca

@p5pRT
Copy link
Author

p5pRT commented Jan 24, 2005

From @rgs

Yitzchak Scott-Thoennes wrote​:

My last try at arguing against this; hereafter I'll stay quiet.
The following​:

system q!echo 'sub foo{"bar"} die'>foo.pl!;
eval q!require "foo.pl"!;
print &foo();
delete $INC{"foo.pl"};
system q!echo 'sub foo{"baz"} 1'>foo.pl!;
require "foo.pl";
print &foo();

prints barbaz without this patch but​:
barCompilation failed in require at req.pl line 6.

with it. I really don't see a way to cache failures in %INC without
what I consider an ugly amount of backward incompatibility.

OK. I've now reverted change 23843.

If this is really, truly, necessary, cache failures in %^NOINC instead
of %INC, and document it in perldelta for 5.10.

I don't see the point of caching failures in yet another special variable
if they're not used by the require() mechanism.

@p5pRT
Copy link
Author

p5pRT commented Jan 24, 2005

From rick@bort.ca

On Mon, Jan 24, 2005 at 02​:42​:43PM +0100, Rafael Garcia-Suarez wrote​:

Yitzchak Scott-Thoennes wrote​:

If this is really, truly, necessary, cache failures in %^NOINC instead
of %INC, and document it in perldelta for 5.10.

I don't see the point of caching failures in yet another special variable
if they're not used by the require() mechanism.

I think the point would be that require() would start checking %^NOINC.
So

  die "Compilation failed in require" if $^NOINC{$file};
  return 1 if $INC{$file};
  _load();

I originally considered implementing it this way but didn't know how to
set up a PL_noincgv analogous to PL_incgv.

But now I think it should stay the way it is (caching as undef) and this
bug should just be fixed on its own. Caching as undef allows some
pretty simple semantics​:

  # Is $file loaded?
  $INC{$file};
 
  # Have we tried to load $file?
  exists $INC{$file};
 
  # What files are loaded?
  grep defined($_), values %INC;

  # Force reload of $file​:
  delete $INC{$file};
  require $file;

--
Rick Delaney
rick@​bort.ca

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