[META] Regressions introduced by refactoring #10555

p5pRT · 2010-08-18T09:37:47Z

Migrated from rt.perl.org#77300 (status was 'open')

Searchable as RT77300$

p5pRT · 2010-08-18T09:37:48Z

From @nwc10

This ticket is a meta-ticket for collating which regressions have been
introduced as a result of refactoring the perl interpreter.

It's unlikely ever to be comprehensive. The intent is that if one finds that
a bug is a regression introduced by a change intended to be a pure
refactoring, link it to this ticket. This way, over time, we can get an idea
of which types of refactoring are safe, and which are dangerous and hence
need more careful checking.

p5pRT · 2010-08-18T10:13:22Z

From @nwc10

Note also

3d4dd4c fixes 7 errors
cf684db fixes 5 errors

3c6f43c to fix an error introduced by
890ce7a
f3dc127 to fix an error introduced by
aec46f1

[might have those two above transposed, but the pattern is obvious]

8074533 to fix an error introduced by
1df7014

7536ed5 to fix an error introduced by
46c461b

bb5dd93 to fix an error introduced by
890ce7a

b88ec9b to fix an error introduced by
66ceb53, fixed in
4f58fed, repeated by
1eb6e4c

52a5bfa to fix an error introduced by
7827dc6

a3e405b to fix an error introduced by
890ce7a

1875938 is *not* from a refactoring error.

b439006 fixes 3 bugs introduced by
aec46f1

3bdcbd2 to fix a minor error

p5pRT · 2010-08-18T10:13:23Z

@nwc10 - Status changed from 'new' to 'open'

p5pRT · 2011-03-09T03:47:17Z

From @tonycoz

Refactoring of Tie::Hash::NamedCapture in
f808887 introduced a C++ build
regression fixed in 3b9b32c

p5pRT · 2011-06-06T03:03:28Z

From @cpansprout

These commits:

a130272
e5accad
fb2352e
b2b95e4
47d6f3d
4ab5bd5
a991bd3

fix COW regressions introduced by commit a604c75.

Bugs #75656 and #91834 were caused by the same commit.

p5pRT · 2011-08-22T15:26:49Z

From @cpansprout

This commit:

commit 7a5fd60
Author: Nicholas Clark <nick@ccl4.org>
Date: Fri Jan 7 12:46:07 2005 +0000

Stage 1 of utf8 support for soft references.
Change gv_fetchpv to take a UTF8 flag, as gv_fetchpvn_flags
Add gv_fetchsv to look up a GV by SV rather than a char * pointer
Provide a backwards compatability gv_fetchpv
Migrate from gv_fetchpv to gv_fetchsv where the caller was grabbing
the pointer from an SV
All tests still pass.

p4raw-id: //depot/perl@23766

though not just a refactoring (as it was preparing for a real bug fix),
caused this bug:

$ perl5.8.8 -le 'print defined *{"!"} for 1..2'
1
1
$ perl5.10.0 -le 'print defined *{"!"} for 1..2'

1

(i.e., *{} autovivifying a glob, but not returning it), which was fixed
by 99fc7ec.

p5pRT · 2011-08-23T07:20:16Z

From @cpansprout

On Mon Aug 22 08:26:49 2011, sprout wrote:

This commit:

commit 7a5fd60
Author: Nicholas Clark <nick@ccl4.org>
Date: Fri Jan 7 12:46:07 2005 +0000
Stage 1 of utf8 support for soft references\.
Change gv\_fetchpv to take a UTF8 flag\, as gv\_fetchpvn\_flags
Add gv\_fetchsv to look up a GV by SV rather than a char \* pointer
Provide a backwards compatability gv\_fetchpv
Migrate from gv\_fetchpv to gv\_fetchsv where the caller was grabbing
the pointer from an SV
All tests still pass\.

p4raw\-id&#8203;: //depot/perl@&#8203;23766
though not just a refactoring (as it was preparing for a real bug fix),
caused this bug:

$ perl5.8.8 -le 'print defined *{"!"} for 1..2'
1
1
$ perl5.10.0 -le 'print defined *{"!"} for 1..2'

1

(i.e., *{} autovivifying a glob, but not returning it), which was fixed
by 99fc7ec.

And this one (#97482):

$ perl -we 'close undef'
Use of uninitialized value in ref-to-glob cast at -e line 1.
Use of uninitialized value in ref-to-glob cast at -e line 1.

p5pRT · 2011-09-30T16:34:41Z

From @cpansprout

Here’s an interesting regression:

----Program----
@ISA = foo; package foo; sub onon; use overload "+" => "onon";
"main"->${\"(+"}; bless [], ""

----Output of .../pMUxm3P/perl-5.8.0@24524/bin/perl----
Stub found while resolving method `???' overloading `+' in package
`main' at test line 1.
Stub found while resolving method `???' overloading `+' in package `main'.

----EOF ($?='65280')----
----Output of .../pd1navr/perl-5.8.0@24529/bin/perl----
Stub found while resolving method `???' overloading `+' in package
`overload' at test line 1.
Stub found while resolving method `???' overloading `+' in package
`overload'.

----EOF ($?='65280')----

Notice how the package name has become ‘overload’.

It was caused by:

commit bfcb351
Author: Nicholas Clark <nick@ccl4.org>
Date: Sat May 21 12:31:52 2005 +0000

Move hv_name, hv_eiter and hv_riter into a new aux structure.
Provide (more efficient) _get and _set macros.
Adjust the core to use them.

p4raw-id: //depot/perl@24526

Specifically, this part, which uses the new hvname variable to replace
both HvNAME(stash) *and* HvNAME(GvSTASH(CvGV(cv))):

@@ -1404,8 +1408,9 @@ Perl_Gv_AMupdate(pTHX_ HV *stash)
gv = Perl_gv_fetchmeth(aTHX_ stash, cooky, l, -1);
cv = 0;
if (gv && (cv = GvCV(gv))) {
+ const char *hvname;
if (GvNAMELEN(CvGV(cv)) == 3 && strEQ(GvNAME(CvGV(cv)), "nil")
- && strEQ(HvNAME(GvSTASH(CvGV(cv))), "overload")) {
+ && strEQ(hvname = HvNAME_get(GvSTASH(CvGV(cv))), "overload")) {
/* This is a hack to support autoloading..., while
knowing *which* methods were declared as overloaded. */
/* GvSV contains the name of the method. */
@@ -1413,7 +1418,7 @@ Perl_Gv_AMupdate(pTHX_ HV *stash)

DEBUG_o( Perl_deb(aTHX_ "Resolving method `%"SVf256\
"' for overloaded `%s' in package `%.256s'\n",
- GvSV(gv), cp, HvNAME(stash)) );
+ GvSV(gv), cp, hvname) );
if (!SvPOK(GvSV(gv))
|| !(ngv = gv_fetchmethod_autoload(stash, SvPVX_const(GvSV(gv)),
FALSE)))
@@ -1425,12 +1430,12 @@ Perl_Gv_AMupdate(pTHX_ HV *stash)
"in package `%.256s'",
(GvCVGEN(gv) ? "Stub found while resolving"
: "Can't resolve"),
- name, cp, HvNAME(stash));
+ name, cp, hvname);
}
cv = GvCV(gv = ngv);
}
DEBUG_o( Perl_deb(aTHX_ "Overloading `%s' in package `%.256s' via
`%.256s::%.256s' \n",
- cp, HvNAME(stash), HvNAME(GvSTASH(CvGV(cv))),
+ cp, HvNAME_get(stash), HvNAME_get(GvSTASH(CvGV(cv))),
GvNAME(CvGV(cv))) );
filled = 1;
if (i < DESTROY_amg)

p5pRT · 2011-10-01T03:19:09Z

From @cpansprout

On Fri Sep 30 09:34:41 2011, sprout wrote:

Here’s an interesting regression:

----Program----
@ISA = foo; package foo; sub onon; use overload "+" => "onon";
"main"->${\"(+"}; bless [], ""

----Output of .../pMUxm3P/perl-5.8.0@24524/bin/perl----
Stub found while resolving method `???' overloading `+' in package
`main' at test line 1.
Stub found while resolving method `???' overloading `+' in package `main'.

----EOF ($?='65280')----
----Output of .../pd1navr/perl-5.8.0@24529/bin/perl----
Stub found while resolving method `???' overloading `+' in package
`overload' at test line 1.
Stub found while resolving method `???' overloading `+' in package
`overload'.

----EOF ($?='65280')----

Notice how the package name has become ‘overload’.

It was caused by:

commit bfcb351
Author: Nicholas Clark <nick@ccl4.org>
Date: Sat May 21 12:31:52 2005 +0000

and fixed by f0e9f18.

p5pRT · 2011-10-16T23:09:25Z

From @cpansprout

This commit:

commit bf53b3a
Author: Nicholas Clark <nick@ccl4.org>
Date: Mon Feb 20 11:54:38 2006 +0000

xcv_start and xcv_xsubany can be merged into a union, as they are never
both needed.

p4raw-id: //depot/perl@27243

caused two regressions (sort(xsub 1,2,3) ignoring the xsub and
sort(constant 1,2,3) crashing), which were fixed by:

commit 2fc49ef
Author: Father Chrysostomos <sprout@cpan.org>
Date: Sat Oct 15 14:05:33 2011 -0700

Make XS sort routines work again

p5pRT · 2011-11-29T21:50:30Z

From @cpansprout

This commit:

commit 66a1b24
Author: Andy Lester <andy@petdance.com>
Date: Mon Jun 6 05:11:07 2005 -0500

Random cleanups #47
Message-ID: <20050606151107.GC7022@petdance.com>

p4raw-id: //depot/perl@24735

made newXS trigger warning only when replacing a subroutine in the
autouse namespace. I.e., it reversed the logic.

This commit fixed that, among other things:

commit 799fd3b
Author: Father Chrysostomos <sprout@cpan.org>
Date: Sun Nov 20 23:46:48 2011 -0800

Restore autouse’s exemption from redef warnings

p5pRT · 2012-04-18T08:15:25Z

From @nwc10

This commit

commit de009b7
Author: Andy Lester <andy@petdance.com>
Date: Wed Apr 27 05:02:43 2005 -0500

perlio-two.patch: More warnings squashed, more consts
Message-ID: <20050427150243.GA21883@petdance.com>

p4raw-id: //depot/perl@24338

amongst many things, did this:

@@ -413,8 +415,8 @@ PerlIO_importFILE(FILE *stdio, const char *mode)
FILE *
PerlIO_findFILE(PerlIO *pio)
{
- int fd = PerlIO_fileno(pio);
- FILE *f = fdopen(fd, "r+");
+ const int fd = PerlIO_fileno(pio);
+ FILE * const f = fdopen(fd, "r+");
PerlIO_flush(pio);
if (!f && errno == EINVAL)
f = fdopen(fd, "w");

which isn't going to fly. (But is in sfio-only code)

This commit

commit 4ee3916
Author: Chip Salzenberg <chip@pobox.com>
Date: Wed Nov 26 15:01:41 2008 -0800

standardize save/restore of errno & vaxc$errno
Message-ID: <20081127070141.GD17663@tytlal.topaz.cx>

p4raw-id: //depot/perl@35018

made a typo in some sfio-only code, which wasn't spotted.

Both fixed in the branch nicholas/misc-tidyup (not yet merged or deleted)

p5pRT · 2012-07-27T17:55:46Z

From @cpansprout

This commit:

commit 437388a
Author: Nicholas Clark <nick@ccl4.org>
Date: Thu Nov 18 14:54:44 2010 +0000

Refactor newATTRSUB()'s logic for grafting a sub definition to an
existing stub

Previously it was using cv_undef() to (partially) free the target CV
(the
pre-existing stub), before donating it the padlist and outside
pointers from
the source CV (the definition, just compiled), and then freeing up
the remains
of the source CV.

Instead, explicitly exchange padlist and outside pointers,
explicitly assign
other fields that need changing (file and stash), and assert that
various
CvFLAGS are as we expect them.

causes this:

$ ./perl -Ilib -e '\&f; *bar = *f; eval "sub bar {}"'
Assertion failed: (CvGV(cv) == gv), function Perl_newATTRSUB_flags, file
op.c, line 7464.
Abort trap

(fixed by f6894bc)

and this:

$ ./perl -Ilib -e '*f=sub{}; undef &f; *bar = *f; eval "sub bar {}"'
Assertion failed: (!CvWEAKOUTSIDE(cv)), function Perl_newATTRSUB_flags,
file op.c, line 7462.
Abort trap

(fixed by e52de15, although the assertion failure actually exposed
an earlier problem caused by 5c41a5f, which was a bug fix, not a
refactoring [see e52de15 for detail]).

--

Father Chrysostomos

p5pRT · 2012-10-28T21:43:18Z

From @cpansprout

Commit b9ad13a omitted a break; in sv_upgrade, resulting in an
assertion failure in ‘$x = 1.1; $x = ${qr//}’ fixed by 12c45b2.

It also set SvSTASH to null in reg_temp_copy, resulting in a crash for:

bless \$x;
$x = ${qr//}; # SvOBJECT set, but no SvSTASH
print \$x; # crash

Commit 703c388 did the same thing with SvMAGIC, causing $!=${qr//} to
clobber magic.

Those last two issue were fixed by 78a84e4.

Those two commits were not just regressions, but bug fixes. However,
they were both part of larger refactoring (fixing bugs introduced
earlier in the same refactoring) to make regexps first-class SVs.

f082678 caused this to leak the PV:

$x = "hello"; $x = ${qr//}

That was likewise a bug fix, but still part of a larger refactoring.

I fixed that in edd9fea.

--

Father Chrysostomos

p5pRT · 2013-08-20T07:00:40Z

From @cpansprout

Removing the link to
http://www.nntp.perl.org/group/perl.perl5.porters/2011/02/msg169504.html.
I thought aelemfast was a recent addition.

nwc10 · 2021-10-18T08:21:15Z

0b3da58 fixes a double free introduced as part of 6136c70

p5pRT added Severity Low type-meta labels Oct 18, 2019

toddr added the meta-refactor-regressions label Oct 25, 2019

xenu removed the Severity Low label Dec 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[META] Regressions introduced by refactoring #10555

[META] Regressions introduced by refactoring #10555

p5pRT commented Aug 18, 2010

p5pRT commented Aug 18, 2010

Uh oh!

p5pRT commented Aug 18, 2010

Uh oh!

p5pRT commented Aug 18, 2010

Uh oh!

p5pRT commented Mar 9, 2011

Uh oh!

p5pRT commented Jun 6, 2011

Uh oh!

p5pRT commented Aug 22, 2011

Uh oh!

p5pRT commented Aug 23, 2011

Uh oh!

p5pRT commented Sep 30, 2011

Uh oh!

p5pRT commented Oct 1, 2011

Uh oh!

p5pRT commented Oct 16, 2011

Uh oh!

p5pRT commented Nov 29, 2011

Uh oh!

p5pRT commented Apr 18, 2012

Uh oh!

p5pRT commented Jul 27, 2012

Uh oh!

p5pRT commented Oct 28, 2012

Uh oh!

p5pRT commented Aug 20, 2013

Uh oh!

nwc10 commented Oct 18, 2021

Uh oh!

[META] Regressions introduced by refactoring #10555

[META] Regressions introduced by refactoring #10555

Comments

p5pRT commented Aug 18, 2010

p5pRT commented Aug 18, 2010

From @nwc10

Uh oh!

p5pRT commented Aug 18, 2010

From @nwc10

Uh oh!

p5pRT commented Aug 18, 2010

Uh oh!

p5pRT commented Mar 9, 2011

From @tonycoz

Uh oh!

p5pRT commented Jun 6, 2011

From @cpansprout

Uh oh!

p5pRT commented Aug 22, 2011

From @cpansprout

Uh oh!

p5pRT commented Aug 23, 2011

From @cpansprout

Uh oh!

p5pRT commented Sep 30, 2011

From @cpansprout

Uh oh!

p5pRT commented Oct 1, 2011

From @cpansprout

Uh oh!

p5pRT commented Oct 16, 2011

From @cpansprout

Uh oh!

p5pRT commented Nov 29, 2011

From @cpansprout

Uh oh!

p5pRT commented Apr 18, 2012

From @nwc10

Uh oh!

p5pRT commented Jul 27, 2012

From @cpansprout

Uh oh!

p5pRT commented Oct 28, 2012

From @cpansprout

Uh oh!

p5pRT commented Aug 20, 2013

From @cpansprout

Uh oh!

nwc10 commented Oct 18, 2021

Uh oh!