On Fri, Oct 31, 2008 at 8:37 AM, Tony Cook<tony@develop-help.com> wrote: > On Fri, Oct 31, 2008 at 01:25:23PM +0000, Nicholas Clark wrote: >> On Sat, Nov 01, 2008 at 12:20:21AM +1100, Tony Cook wrote: >> > The following patch fixed the problem with the sample mro.pl. >> > >> > The extra test in t/mro/basic.t fails without the change to sv.c >> >> Thanks for this. >> >> Without looking fully at it yet (something I don't currently have time for, >> sorry) a bikeshed question: >> >> > + I32 mro_changes = 0; >> >> > + if (strEQ(GvNAME((GV*)dstr), "ISA")) >> > + mro_changes = 2; >> >> > + if(mro_changes == 2) mro_isa_changed_in(GvSTASH(dstr)); >> > return; >> > } >> >> Why the value 2, for a new variable that is effectively only used for >> true/false? > > I was following the similar logic in S_glob_assign_glob and > considering whether similar code was needed here. > > I'm not all that attached to the value 2, using it as a boolean would > work just as well. > > I just tried the test script from 60232 and that runs successfully > with the patch. This is now (at long last) in blead at: http://perl5.git.perl.org/perl.git/commitdiff/26d68d8 I see no new test failures in the core test suite with default builds on recent versions of Mac OS X or VMS, but folks who do evil things with mro method caches might want to check their back closets for appropriate torture tests. And of course follow the smoke reports at: http://www.nntp.perl.org/group/perl.daily-build.reports/Thread Previous