[perl #75176] Symbol::delete_package does not free certain memory associated with package::ISA

This commit makes @ISA changes and package aliasing update PL_isarev
properly, removing old, unnecessary entries in addition to adding new
entries. So now it is capable of shrinking, not just growing.

------------
Gory Details
------------

There is a chicken-and-egg problem when it comes to calling
mro_isa_changed_in on the affected classes: When an isa linearisation
is recalculated, it uses the existing linearisations of the super-
classes (if any) (or at least the DFS implementation does). Since an
assigned package (e.g., the *b:: in *a:: = *b::) can contain nested
packages that inherit from each other in any order (b::c isa b::c::d
or b::c::e isa b::c), this means that mro_isa_changed_in *must not* be
called on any stash while another stash contains stale data.

So mro_package_moved has been restructured. It is no longer recurs-
ive. The recursive code for iterating through nested stashes has been
moved into a separate, static routine: mro_gather_and_rename. Instead
of calling mro_isa_changed_in during the iteration, it adds all the
classes to ‘the big hash’, which mro_package_moved holds a pointer to.
When mro_gather_and_rename returns, mro_package_moved iterates through
the big hash twice: the first time to wipe caches; the second to call
mro_isa_changed_in on all the stashes.

This ‘big hash’ is now used in place of the seen_stashes that
mro_package_moved used before.

Both mro_package_moved and mro_isa_changed_in now use the existing
mrometa->isa hash to determine which classes used to be superclasses
of the stash in question. A separate routine, S_mro_clean_isarev,
deletes entries mention in isa, except for those that still exist in
the new isa hash.

mro_isa_changed_in now does two iterations through isarev, just like
mro_package_moved. It has to call get_linear_isa on the subclasses so
that it can see what is in the new meta->isa hash created thereby.
Consequently, it has to make sure that all the subclasses have their
caches deleted before it can update anything. It makes the same
changes to isarev for each subclass that are made further down on the
class for which mro_isa_changed_in was called. Yes, it is repetitive.
But calling mro_isa_changed_in recursively has more overhead and would
do more unnecessary work. (Maybe we could make some macros for this
repetitive code.)

The loop through the superclasses near the end of mro_isa_changed_in
no longer adds the subclasses to all the superclasses’ isarev hashes,
because that is taken care of further up.

------------
Side Effects
------------

One result of this change is that mro::is_universal no longer
returns true for classes that are no longer universal. I consider
that a bug fix.

-------------
Miscellaneous
-------------

This also removes obsolete comments in mro_isa_changed_in, concerning
fake and universal flags on stashes, that have been invalid since
dd69841bebe.

3 files changed, 35 insertions, 8 deletions

diff --git a/t/mro/basic.t b/t/mro/basic.t
index f49695607e..353352bb61 100644
--- a/t/mro/basic.t
+++ b/t/mro/basic.t
@@ -57,7 +57,7 @@ ok(!mro::is_universal('MRO_B'));
 ok(mro::is_universal('MRO_B'));
 
 @UNIVERSAL::ISA = ();
-ok(mro::is_universal('MRO_B'));
+ok(!mro::is_universal('MRO_B'));
 
 # is_universal, get_mro, and get_linear_isa should
 # handle non-existant packages sanely
diff --git a/t/mro/isarev.t b/t/mro/isarev.t
index 01b0017b0b..c76cf9e084 100644
--- a/t/mro/isarev.t
+++ b/t/mro/isarev.t
@@ -10,7 +10,7 @@ BEGIN {
 
 use strict;
 use warnings;
-plan(tests => 15);
+plan(tests => 20);
 
 use mro;
 
@@ -25,7 +25,21 @@ sub i {
  goto &is;
 }
 
-$::TODO = "[perl #75176] isarev leeks (and onions)";
+# Basic isarev updating, when @ISA changes
+@Pastern::ISA = "BodyPart::Ungulate";
+@Scur::ISA    = "BodyPart::Ungulate";
+@BodyPart::Ungulate::ISA = "BodyPart";
+i BodyPart => qw [ BodyPart::Ungulate Pastern Scur ],
+ 'subclasses and subsubclasses are added to isarev';
+@Pastern::ISA = ();
+i BodyPart => qw [ BodyPart::Ungulate Scur ],
+ 'single deletion from isarev';
+@BodyPart::Ungulate::ISA = ();
+i BodyPart => qw [ ], 'recursive deletion from isarev';
+                      # except underneath it is not actually recursive
+
+
+# More complicated tests that move packages around
 
 @Huskey::ISA = "Dog";
 @Dog::ISA = "Canid";
@@ -70,17 +84,18 @@ i g => qw [ Weird::Thing ],
 *Caprine:: = *Chevre::;
 i"Hoofed::Mammal" => qw[ Caprid ],
  "replacing a stash updates isarev entries";
-i Caprine => qw[ Whatever ],
+i Chevre => qw[ Caprid::Dairy Whatever ],
  "replacing nested stashes updates isarev entries";
 
 @Disease::Eye::ISA = "Disease";
 @Disease::Eye::Infectious::ISA = "Disease::Eye";
 @Keratoconjunctivitis::ISA = "Disease::Ophthalmic::Infectious";
 *Disease::Ophthalmic:: = *Disease::Eye::;
+{package some_random_new_symbol::Infectious} # autovivify
 *Disease::Ophthalmic:: = *some_random_new_symbol::;
-i Disease => qw[ Disease::Eye ],
+i Disease => qw[ Disease::Eye Disease::Eye::Infectious ],
  "replacing an alias of a stash updates isarev entries";
-i Caprine => qw[ Disease::Eye ],
+i"Disease::Eye" => qw[ Disease::Eye::Infectious ],
  "replacing an alias of a stash containing another updates isarev entries";
 i"some_random_new_symbol::Infectious" => qw[ Keratoconjunctivitis ],
  "replacing an alias updates isarev of stashes nested in the replacement";
@@ -93,5 +108,17 @@ undef *Empty::;
 {package Zilch::Empty} # autovivify it
 *Empty:: = *Zilch::;
 i Zilch => qw[ Null ], "assigning to an empty spot updates isarev";
-i"Zilch::Empty" => qw[ Null::Empty ],
+i"Zilch::Empty" => qw[ Null::Null ],
  "assigning to an empty spot updates isarev of nested packages";
+
+# Classes inheriting from multiple classes that get moved in a single
+# assignment.
+@foo::ISA = ("B", "B::B");
+{package A::B}
+my $A = \%A::;     # keep a ref
+*A:: = 'whatever'; # clobber it
+*B:: = $A;         # assign to two superclasses of foo at the same time
+# There should be no A::B isarev entry.
+i"A::B" => qw [], 'assigning to two superclasses at the same time';
+ok !foo->isa("A::B"),
+ "A class must not inherit from its superclass’s former name";
diff --git a/t/op/universal.t b/t/op/universal.t
index d74686f97d..db79dcde70 100644
--- a/t/op/universal.t
+++ b/t/op/universal.t
@@ -298,7 +298,7 @@ use warnings "deprecated";
     @RT66112::B::ISA = qw//;
     @RT66112::C::ISA = qw/RT66112::B/;
     @RT66112::T3::ISA = qw/RT66112::C/;
-    ok(RT66112::T3->isa('RT66112::A'), "modify \@ISA in isa (RT66112::T3 isa RT66112::A)");
+    ok(RT66112::T3->isa('RT66112::A'), "modify \@ISA in isa (RT66112::T3 isa RT66112::A)") or require mro, diag "@{mro::get_linear_isa('RT66112::T3')}";
 
     @RT66112::E::ISA = qw/RT66112::D/;
     @RT66112::T4::ISA = qw/RT66112::E/;