Skip to content

Commit 177bd8b

Browse files
committed
class.c: clean up any state if we don't finish the class
Fixes #22169
1 parent b786e5e commit 177bd8b

File tree

4 files changed

+126
-2
lines changed

4 files changed

+126
-2
lines changed

MANIFEST

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5970,6 +5970,7 @@ t/class/class.t See if class declarations work
59705970
t/class/construct.t See if class constructors work
59715971
t/class/destruct.t See if class destruction works
59725972
t/class/field.t See if class field declarations work
5973+
t/class/gh22169.t Test defining a class that previously failed to define
59735974
t/class/inherit.t See if class inheritance works
59745975
t/class/method.t See if class method declarations work
59755976
t/class/phasers.t See if class phaser blocks work

class.c

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -805,14 +805,78 @@ Perl_class_seal_stash(pTHX_ HV *stash)
805805
aux->xhv_class_initfields_cv = initfields;
806806
}
807807
else {
808-
/* we had errors, clean up and don't populate initfields */
808+
/* we had errors, clean up */
809+
810+
SvREFCNT_dec(aux->xhv_class_superclass);
811+
aux->xhv_class_superclass = NULL;
812+
813+
/* clean up adjust blocks */
814+
SvREFCNT_dec(aux->xhv_class_adjust_blocks);
815+
aux->xhv_class_adjust_blocks = NULL;
816+
817+
/* name to slot index */
818+
SvREFCNT_dec(aux->xhv_class_param_map);
819+
aux->xhv_class_param_map = NULL;
820+
821+
/* clean up the ops for defaults for fields, if any, since
822+
padname_free() doesn't.
823+
*/
809824
PADNAMELIST *fieldnames = aux->xhv_class_fields;
810825
if (fieldnames) {
811826
for(SSize_t i = PadnamelistMAX(fieldnames); i >= 0 ; i--) {
812827
PADNAME *pn = PadnamelistARRAY(fieldnames)[i];
813828
op_free(PadnameFIELDINFO(pn)->defop);
829+
PadnameFIELDINFO(pn)->defop = NULL;
830+
}
831+
PadnamelistREFCNT_dec(fieldnames);
832+
aux->xhv_class_fields = NULL;
833+
}
834+
835+
/* clean up methods */
836+
/* should we keep a separate list of these instead? */
837+
if (hv_iterinit(stash)) {
838+
HE *he;
839+
while ((he = hv_iternext(stash)) != NULL) {
840+
STRLEN klen;
841+
const char * const kpv = HePV(he, klen);
842+
SV *entry = HeVAL(he);
843+
CV *cv = NULL;
844+
if (SvTYPE(entry) == SVt_PVGV
845+
&& (cv = GvCV((GV*)entry))
846+
&& (CvIsMETHOD(cv) || memEQs(kpv, klen, "new"))) {
847+
SvREFCNT_dec(cv);
848+
GvCV_set((GV*)entry, NULL);
849+
}
850+
else if (SvTYPE(entry) == SVt_PVCV
851+
&& (CvIsMETHOD((CV*)entry) || memEQs(kpv, klen, "new"))) {
852+
(void)hv_delete(stash, kpv, HeUTF8(he) ? -(I32)klen : (I32)klen,
853+
G_DISCARD);
854+
}
814855
}
856+
++PL_sub_generation;
857+
}
858+
859+
/* field clean up */
860+
resume_compcv_final(aux->xhv_class_suspended_initfields_compcv);
861+
SvREFCNT_dec(PL_compcv);
862+
Safefree(aux->xhv_class_suspended_initfields_compcv);
863+
aux->xhv_class_suspended_initfields_compcv = NULL;
864+
865+
/* remove any ISA entries */
866+
SV *isaname = newSVpvf("%" HEKf "::ISA", HvNAME_HEK(stash));
867+
sv_2mortal(isaname);
868+
AV *isa = get_av(SvPV_nolen(isaname), (SvFLAGS(isaname) & SVf_UTF8));
869+
if (isa) {
870+
/* we make this read-only above since class-keyword
871+
classes manage ISA themselves, the class has failed to
872+
load, so we no longer manage it.
873+
*/
874+
SvREADONLY_off((SV *)isa);
875+
av_clear(isa);
815876
}
877+
878+
/* no longer a class */
879+
aux->xhv_aux_flags &= ~HvAUXf_IS_CLASS;
816880
}
817881
}
818882

t/class/gh22169.t

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#!./perl
2+
3+
BEGIN {
4+
chdir 't' if -d 't';
5+
require './test.pl';
6+
set_up_inc('../lib');
7+
require Config;
8+
}
9+
10+
use v5.36;
11+
use feature 'class';
12+
no warnings 'experimental::class';
13+
14+
class Base {
15+
method g() { "Base" }
16+
ADJUST {
17+
::fail("original Base ADJUST block should not be called");
18+
}
19+
}
20+
21+
class Base2 {
22+
method g() { "Base2" }
23+
}
24+
25+
BEGIN {
26+
our $saw_end;
27+
eval <<'CLASS';
28+
class MyTest :isa(Base) {
29+
field $x = "First";
30+
field $w :reader;
31+
ADJUST {
32+
fail("ADJUST from failed class definition called");
33+
}
34+
method f () { $x }
35+
method h() { }
36+
method z() { }
37+
# make sure some error above doesn't invalidate the test, this
38+
BEGIN { ++$saw_end; }
39+
CLASS
40+
ok($saw_end, "saw the end of the incomplete class definition");
41+
}
42+
43+
class MyTest :isa(Base2) {
44+
field $y = "Second";
45+
method f() { $y }
46+
ADJUST {
47+
::pass("saw adjust in replacement class definition");
48+
}
49+
}
50+
51+
my $z = new_ok("MyTest");
52+
ok(!$z->can("h"), "h() should no longer be present");
53+
isa_ok($z, "Base2", "check base class");
54+
is($z->g(), "Base2", "Base class correct via g");
55+
is($z->f(), "Second", "f() value");
56+
ok(!$z->can("w"), 'accessor for $w removed');
57+
58+
done_testing();
59+

t/lib/croak/class

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ no warnings 'experimental::class';
145145
eval "class C {";
146146
C->new;
147147
EXPECT
148-
Cannot create an object of incomplete class "C" at - line 5.
148+
Can't locate object method "new" via package "C" at - line 5.
149149
########
150150
# NAME try to create an object of incomplete class (compile-time)
151151
use v5.36;

0 commit comments

Comments
 (0)