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

sub refs returned from @INC hooks receive wrong parameters #12569

Closed
p5pRT opened this issue Nov 16, 2012 · 11 comments
Closed

sub refs returned from @INC hooks receive wrong parameters #12569

p5pRT opened this issue Nov 16, 2012 · 11 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 16, 2012

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

Searchable as RT115754$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 16, 2012

From @haarg

Created by @haarg

@​INC hooks can return a filehandle, a subroutine reference, and an
optional state value. The subroutine will then be called to act
as either a filter or to directly feed the contents to be loaded.

The documentation for require specifies that this sub will receive
a reference to itself, and the optional state. Instead, it is sent
the value 0 and the optional state.

$ cat require-feed-sub.pl
use strict;
use warnings;

unshift @​INC, sub {
  return (sub {
  print "got -@​_-\n";
  $_ = "1";
  return 0;
  }, 'extra');
};

require test;
$ perl require-feed-sub.pl
got -0 extra-

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.12.4:

Configured by _mdnsresponder at Wed Jun 20 13:42:53 PDT 2012.

Summary of my perl5 (revision 5 version 12 subversion 4) configuration:

  Platform:
    osname=darwin, osvers=12.0, archname=darwin-thread-multi-2level
    uname='darwin b1026.apple.com 12.0 darwin kernel version 12.0.0:
tue may 15 23:31:29 pdt 2012; root:xnu-2050.6.70~1release_x86_64
x86_64 '
    config_args='-ds -e -Dprefix=/usr -Dccflags=-g  -pipe  -Dldflags=
-Dman3ext=3pm -Duseithreads -Duseshrplib -Dinc_version_list=none
-Dcc=clang'
    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='clang', ccflags ='-arch i386 -arch x86_64 -g -pipe -fno-common
-DPERL_DARWIN -fno-strict-aliasing -fstack-protector
-I/usr/local/include',
    optimize='-Os',
    cppflags='-g -pipe -fno-common -DPERL_DARWIN -fno-strict-aliasing
-fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.2.1 Compatible Apple Clang 4.0
(tags/Apple/clang-418.0.60)', 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='clang -mmacosx-version-min=10.8', ldflags ='-arch i386 -arch
x86_64 -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib
    libs=-ldbm -ldl -lm -lutil -lc
    perllibs=-ldl -lm -lutil -lc
    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 -L/usr/local/lib -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.12.4:
    /Library/Perl/5.12/darwin-thread-multi-2level
    /Library/Perl/5.12
    /Network/Library/Perl/5.12/darwin-thread-multi-2level
    /Network/Library/Perl/5.12
    /Library/Perl/Updates/5.12.4
    /System/Library/Perl/5.12/darwin-thread-multi-2level
    /System/Library/Perl/5.12
    /System/Library/Perl/Extras/5.12/darwin-thread-multi-2level
    /System/Library/Perl/Extras/5.12
    .


Environment for perl 5.12.4:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/gknop
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/X11/bin
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/bash

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @jkeenan

On Thu Nov 15 19​:26​:41 2012, haarg wrote​:

This is a bug report for perl from haarg@​haarg.org,
generated with the help of perlbug 1.39 running under perl 5.12.4.

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

@​INC hooks can return a filehandle, a subroutine reference, and an
optional state value. The subroutine will then be called to act
as either a filter or to directly feed the contents to be loaded.

The documentation for require specifies that this sub will receive
a reference to itself, and the optional state. Instead, it is sent
the value 0 and the optional state.

I don't claim to understand these @​INC hooks very well, but the documentation for 'require' in perl-5.22.0 seems to me to be saying something different from what you are claiming​:

#####
Subroutine references are the simplest case [of @​INC hooks]. When the inclusion system walks through @​INC and encounters a subroutine, this subroutine gets called with two parameters, the first a reference to itself, and the second the name of the file to be included (e.g., "Foo/Bar.pm").
#####

You state that the second argument is "the optional state." The documentation says the second argument should be a file name.

Can you clarify?

Thank you very much.

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @cowens

The part you are quoting is for the first layer of subroutines. The
part he is talking about is in the return value stuff​:

  Subroutine references are the simplest case. When the inclusion
  system walks through @​INC and encounters a subroutine, this
  subroutine gets called with two parameters, the first a reference
  to itself, and the second the name of the file to be included
  (e.g., "Foo/Bar.pm"). The subroutine should return either nothing
  or else a list of up to four values in the following order​:

  1 A reference to a scalar, containing any initial source code to
  prepend to the file or generator output.

  2 A filehandle, from which the file will be read.

  3 A reference to a subroutine. If there is no filehandle
  (previous item), then this subroutine is expected to generate
  one line of source code per call, writing the line into $_ and
  returning 1, then finally at end of file returning 0. If there
  is a filehandle, then the subroutine will be called to act as
  a simple source filter, with the line as read in $_. Again,
  return 1 for each valid line, and 0 after all lines have been
  returned.

  4 Optional state for the subroutine. The state is passed in as
  $_[1]. A reference to the subroutine itself is passed in as
  $_[0].

Specifically, return values 3 and 4. The coderef returned should get
a reference to itself plus the optional state. That subroutine should
generate a line of code per call and return 0 or 1 depending on
whether it is done or not. The docs have not changed since at least
5.6.0 (except for the addition of the scalar reference return option
at some point), but the behavior is the same from 5.6.0 to today​: the
first argument is 0 and the second is the state.

I am not sure why the documentation and behavior seem to be out of
sync, but it has been this way for a long time without anyone caring;
and to be honest, I am not 100% sure what the use case is.

On Sat, Aug 1, 2015 at 9​:27 PM, James E Keenan via RT
<perlbug-followup@​perl.org> wrote​:

On Thu Nov 15 19​:26​:41 2012, haarg wrote​:

This is a bug report for perl from haarg@​haarg.org,
generated with the help of perlbug 1.39 running under perl 5.12.4.

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

@​INC hooks can return a filehandle, a subroutine reference, and an
optional state value. The subroutine will then be called to act
as either a filter or to directly feed the contents to be loaded.

The documentation for require specifies that this sub will receive
a reference to itself, and the optional state. Instead, it is sent
the value 0 and the optional state.

I don't claim to understand these @​INC hooks very well, but the documentation for 'require' in perl-5.22.0 seems to me to be saying something different from what you are claiming​:

#####
Subroutine references are the simplest case [of @​INC hooks]. When the inclusion system walks through @​INC and encounters a subroutine, this subroutine gets called with two parameters, the first a reference to itself, and the second the name of the file to be included (e.g., "Foo/Bar.pm").
#####

You state that the second argument is "the optional state." The documentation says the second argument should be a file name.

Can you clarify?

Thank you very much.

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

---
via perlbug​: queue​: perl5 status​: new
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115754

--
Chas. Owens
http​://github.com/cowens
The most important skill a programmer can have is the ability to read.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @jkeenan

On Sat Aug 01 21​:38​:32 2015, cowens wrote​:

The part you are quoting is for the first layer of subroutines. The
part he is talking about is in the return value stuff​:

[snip]

Specifically, return values 3 and 4. The coderef returned should get
a reference to itself plus the optional state. That subroutine should
generate a line of code per call and return 0 or 1 depending on
whether it is done or not. The docs have not changed since at least
5.6.0 (except for the addition of the scalar reference return option
at some point), but the behavior is the same from 5.6.0 to today​: the
first argument is 0 and the second is the state.

I am not sure why the documentation and behavior seem to be out of
sync, but it has been this way for a long time without anyone caring;
and to be honest, I am not 100% sure what the use case is.

Yes, this is starting to sound like a situation where we should change the documentation to match the behavior -- but we should still know what the use case is (or, more likely, was).

Thank you very much.

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @haarg

I think adjusting the docs to match the code is probably the best idea. I only ran into this issue because I was trying to re-implement the behavior of require. I don't see any real value to passing the subref in as a parameter.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @cowens

The actual call to the code is near the end of pp_require in pp_ctl.c​:

  if (filter_sub || filter_cache) {
  /* We can use the SvPV of the filter PVIO itself as our cache, rather
  than hanging another SV from it. In turn, filter_add() optionally
  takes the SV to use as the filter (or creates a new SV if passed
  NULL), so simply pass in whatever value filter_cache has. */
  SV * const fc = filter_cache ? newSV(0) : NULL;
  SV *datasv;
  if (fc) sv_copypv(fc, filter_cache);
  datasv = filter_add(S_run_user_filter, fc);
  IoLINES(datasv) = filter_has_file;
  IoTOP_GV(datasv) = MUTABLE_GV(filter_state);
  IoBOTTOM_GV(datasv) = MUTABLE_GV(filter_sub);
  }

  /* switch to eval mode */
  PUSHBLOCK(cx, CXt_EVAL, SP);
  PUSHEVAL(cx, name);
  cx->blk_eval.retop = PL_op->op_next;

  SAVECOPLINE(&PL_compiling);
  CopLINE_set(&PL_compiling, 0);

  PUTBACK;

  if (doeval(gimme, NULL, PL_curcop->cop_seq, NULL))
  op = DOCATCH(PL_eval_start);
  else
  op = PL_op->op_next;

I can't read the internals well enough to understand what is going on here.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 2, 2015

From @cowens

Yeah, I am just trying to figure out what that 0 represents so the
docs can explain why it is what is. I don't want to document that it
will be 0 and then have the it turn out later it can be some other
value. My best bet (and it isn't a very good one) at the moment is
that it is supposed to be a pointer, but the pointer is NULL, hence
the 0.

I think, and this is a pretty big supposition as well, that this stuff
has something to do with how code filters are implemented.

On Sun, Aug 2, 2015 at 11​:48 AM, Graham Knop via RT
<perlbug-followup@​perl.org> wrote​:

I think adjusting the docs to match the code is probably the best idea. I only ran into this issue because I was trying to re-implement the behavior of require. I don't see any real value to passing the subref in as a parameter.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=115754

--
Chas. Owens
http​://github.com/cowens
The most important skill a programmer can have is the ability to read.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 3, 2015

From @ap

* Graham Knop via RT <perlbug-followup@​perl.org> [2015-08-02 17​:50]​:

I don't see any real value to passing the subref in as a parameter.

And if there’s any then it’s been covered by __SUB__ since 5.16 anyway.
So it’s pointless to change the behaviour anyhow.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 12, 2017

From zefram@fysh.org

The zero argument is just a constant zero, and always
has been ever since this mechanism was introduced in Perl
5.9.4. It has never been meaningful. Documented in commit
0165f7b.

-zefram

@p5pRT p5pRT closed this Dec 12, 2017
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 12, 2017

@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
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.