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

AUTOLOAD :lvalue not working the same in blead as in 5.8.8 #8786

Closed
p5pRT opened this issue Feb 20, 2007 · 8 comments
Closed

AUTOLOAD :lvalue not working the same in blead as in 5.8.8 #8786

p5pRT opened this issue Feb 20, 2007 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 20, 2007

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

Searchable as RT41550$

@p5pRT
Copy link
Author

p5pRT commented Feb 20, 2007

From @smpeters

On IRC, Caelum pointed out that the following example from "Extending
and Embedding Perl" works fine in Perl 5.8.8, but is not working in
blead. The following should print "bar".

  perl -le 'package Foo; sub AUTOLOAD :lvalue { *{$AUTOLOAD} } package main; my
  $foo = bless {},"Foo"; $foo->bar = sub { print "bar" }; $foo->bar;'

Steve Peters
steve@​fisharerojo.org

@p5pRT
Copy link
Author

p5pRT commented Feb 20, 2007

From @andk

On Mon, 19 Feb 2007 20​:58​:23 -0800, Steve Peters (via RT) <perlbug-followup@​perl.org> said​:

  > # New Ticket Created by Steve Peters
  > # Please include the string​: [perl #41550]
  > # in the subject line of all future correspondence about this issue.
  > # <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=41550 >

  > On IRC, Caelum pointed out that the following example from "Extending
  > and Embedding Perl" works fine in Perl 5.8.8, but is not working in
  > blead. The following should print "bar".

  > perl -le 'package Foo; sub AUTOLOAD :lvalue { *{$AUTOLOAD} } package main; my
  > $foo = bless {},"Foo"; $foo->bar = sub { print "bar" }; $foo->bar;'

Binary search hints at 26446.

Change 26446 by nicholas@​nicholas-saigo on 2005/12/22 11​:23​:34

  Add an optimisation to allow proxy constant subroutines to be copied
  as proxy constant subroutines in a new symbol table where possible.
  (Rather than converting them to full blown constant subroutines and
  instantiating 2 typeglobs)

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Feb 20, 2007

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

@p5pRT
Copy link
Author

p5pRT commented Feb 20, 2007

From @nwc10

On Tue, Feb 20, 2007 at 08​:16​:27AM +0100, Andreas J. Koenig wrote​:

On Mon, 19 Feb 2007 20​:58​:23 -0800, Steve Peters (via RT) <perlbug-followup@​perl.org> said​:

perl -le 'package Foo; sub AUTOLOAD :lvalue { *{$AUTOLOAD} } package main; my
$foo = bless {},"Foo"; $foo->bar = sub { print "bar" }; $foo->bar;'

Binary search hints at 26446.

Change 26446 by nicholas@​nicholas-saigo on 2005/12/22 11​:23​:34

Add an optimisation to allow proxy constant subroutines to be copied
as proxy constant subroutines in a new symbol table where possible\.
\(Rather than converting them to full blown constant subroutines and
instantiating 2 typeglobs\)

As I'm about to head off to catch a plane to go to the German Perl Workshop,
I'm unlikely to have any time to look at this before next Monday.

I suspect that it's going to be a "step through with the debugger" job,
with a perl from just either side of that revision number, to see where the
difference in behaviour springs from.

I'm surprised that we don't have a regression test for this construction.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Feb 25, 2007

From @nwc10

On Tue, Feb 20, 2007 at 08​:16​:27AM +0100, Andreas J. Koenig wrote​:

On Mon, 19 Feb 2007 20​:58​:23 -0800, Steve Peters (via RT) <perlbug-followup@​perl.org> said​:

# New Ticket Created by Steve Peters
# Please include the string​: [perl #41550]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=41550 >

On IRC, Caelum pointed out that the following example from "Extending
and Embedding Perl" works fine in Perl 5.8.8, but is not working in
blead. The following should print "bar".

perl -le 'package Foo; sub AUTOLOAD :lvalue { *{$AUTOLOAD} } package main; my
$foo = bless {},"Foo"; $foo->bar = sub { print "bar" }; $foo->bar;'

Binary search hints at 26446.

Change 26446 by nicholas@​nicholas-saigo on 2005/12/22 11​:23​:34

Add an optimisation to allow proxy constant subroutines to be copied
as proxy constant subroutines in a new symbol table where possible\.
\(Rather than converting them to full blown constant subroutines and
instantiating 2 typeglobs\)

Erk. Right. It's happening because this first comment is wrong​:

  /* OP_RV2[AH]V, OP_PAD[AH]V, OP_[AH]ELEM */
#define OPpMAYBE_LVSUB 8 /* We might be an lvalue to return */
  /* OP_PADSV only */

That flag is also applied to OP_RV2GV [via a fall through in a switch
statement in Perl_mod()]

The problem was that based on that comment and other investigation I used
the same bit for the flag

  /* OP_RV2GV only */
#define OPpDONT_INIT_GV 8 /* Call gv_fetchpv with GV_NOINIT */
/* (Therefore will return whatever is currently in the symbol table, not
  guaranteed to be a PVGV) */

Curiously, OPpMAYBE_LVSUB is only used in this part of Perl_peep()

  case OP_PADAV​:
  case OP_GV​:
  if (o->op_type == OP_PADAV || o->op_next->op_type == OP_RV2AV) {
  OP* const pop = (o->op_type == OP_PADAV) ?
  o->op_next : o->op_next->op_next;
  IV i;
  if (pop && pop->op_type == OP_CONST &&
  ((PL_op = pop->op_next)) &&
  pop->op_next->op_type == OP_AELEM &&
  !(pop->op_next->op_private &
  (OPpLVAL_INTRO|OPpLVAL_DEFER|OPpDEREF|OPpMAYBE_LVSUB)) &&
  (i = SvIV(((SVOP*)pop)->op_sv) - CopARYBASE_get(PL_curcop))
  <= 255 &&
  i >= 0)
  {
  GV *gv;
  if (cSVOPx(pop)->op_private & OPpCONST_STRICT)
  no_bareword_allowed(pop);
  if (o->op_type == OP_GV)
  op_null(o->op_next);
  op_null(pop->op_next);
  op_null(pop);
  o->op_flags |= pop->op_next->op_flags & OPf_MOD;
  o->op_next = pop->op_next->op_next;
  o->op_ppaddr = PL_ppaddr[OP_AELEMFAST];
  o->op_private = (U8)i;
  if (o->op_type == OP_GV) {
  gv = cGVOPo_gv;
  GvAVn(gv);
  }
  else
  o->op_flags |= OPf_SPECIAL;
  o->op_type = OP_AELEMFAST;
  }
  o->op_opt = 1;
  break;
  }

I'm not sure what the solution is yet, because I don't know the significance
of the subtleties of that part of Perl_peep(). [Or the subtleties of most of
Perl_peep() for that matter]

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Feb 25, 2007

From rick@bort.ca

On Feb 25 2007, Nicholas Clark wrote​:

Curiously, OPpMAYBE_LVSUB is only used in this part of Perl_peep()

See also LVRET.

[...]

        o\->op\_type = OP\_AELEMFAST;

[...]

This is where OP_AELEM is optimized to OP_AELEMFAST for constant
indices. This can't be done for lvalue subs because you can't know in
advance how it will be used. The only example I can think of is
autovivifying​:

  $a[0]->{whatever};# only OP_AELEM can autovivify $a[0]

but this doesn't work anyway

  sub foo : lvalue { $a[0] }
  foo()->{whatever}; # Can't use an undefined value as a HASH reference

Still, that may be fixed in the future, so I think it is correct to not
optimize to aelemfast in the lvalue sub case.

I'm not sure what the solution is yet, because I don't know the significance
of the subtleties of that part of Perl_peep(). [Or the subtleties of most of
Perl_peep() for that matter]

#define OPpDONT_INIT_GV 32 ?

--
Rick Delaney
rick@​bort.ca

@p5pRT
Copy link
Author

p5pRT commented Feb 26, 2007

From @nwc10

On Sun, Feb 25, 2007 at 05​:32​:07PM -0500, Rick Delaney wrote​:

On Feb 25 2007, Nicholas Clark wrote​:

Curiously, OPpMAYBE_LVSUB is only used in this part of Perl_peep()

See also LVRET.

Thanks for the explanation of why.

#define OPpDONT_INIT_GV 32 ?

I was wondering about 4. I tried 32, and didn't get past dynaloader. Whilst
16 seems to work (no tests fail) comments in op.h and the code of op.c suggests
that that flag value is already possible for OP_RV2GV. So I used 4.

Nicholas Clark

Change 30407 by nicholas@​nicholas-saigo on 2007/02/26 11​:07​:06

  Fix bug #41550 - AUTOLOAD :lvalue not working the same in blead as in
  5.8.8 (a code example from "Extending and Embedding Perl"

Affected files ...

... //depot/perl/op.h#166 edit
... //depot/perl/t/op/sub_lval.t#11 edit

Differences ...

==== //depot/perl/op.h#166 (text) ====

@​@​ -203,14 +203,14 @​@​
#define OPpLVAL_DEFER 16 /* Defer creation of array/hash elem */
  /* OP_RV2?V, OP_GVSV, OP_ENTERITER only */
#define OPpOUR_INTRO 16 /* Variable was in an our() */
- /* OP_RV2[AH]V, OP_PAD[AH]V, OP_[AH]ELEM */
+ /* OP_RV2[AGH]V, OP_PAD[AH]V, OP_[AH]ELEM */
#define OPpMAYBE_LVSUB 8 /* We might be an lvalue to return */
  /* OP_PADSV only */
#define OPpPAD_STATE 16 /* is a "state" pad */
  /* for OP_RV2?V, lower bits carry hints (currently only HINT_STRICT_REFS) */

  /* OP_RV2GV only */
-#define OPpDONT_INIT_GV 8 /* Call gv_fetchpv with GV_NOINIT */
+#define OPpDONT_INIT_GV 4 /* Call gv_fetchpv with GV_NOINIT */
/* (Therefore will return whatever is currently in the symbol table, not
  guaranteed to be a PVGV) */

==== //depot/perl/t/op/sub_lval.t#11 (xtext) ====

@​@​ -3,7 +3,7 @​@​
  @​INC = '../lib';
  require './test.pl';
}
-plan tests=>68;
+plan tests=>69;

sub a : lvalue { my $a = 34; ${\(bless \$a)} } # Return a temporary
sub b : lvalue { ${\shift} }
@​@​ -539,3 +539,14 @​@​

  is($line, "zeroonetwothree");
}
+
+{
+ package Foo;
+ sub AUTOLOAD :lvalue { *{$AUTOLOAD} };
+ package main;
+ my $foo = bless {},"Foo";
+ my $result;
+ $foo->bar = sub { $result = "bar" };
+ $foo->bar;
+ is ($result, 'bar', "RT #41550");
+}

@p5pRT
Copy link
Author

p5pRT commented Apr 30, 2008

p5p@spam.wizbit.be - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant