Bug #21559 for Class-C3: visualize_c3.pl does not add a link

Mon Sep 18 17:28:10 2006 ted [...] tedcarnahan.com - Ticket created

Subject:

visualize_c3.pl does not add a link

Files are attached to RT ticket #21558: Dispatch of methods bug In short, there should be a green link from Fixture::HasSubmission to Fixture::HasStudent, but there is not one in the generated graph. Thanks!

Tue Sep 19 23:56:00 2006 blblack [...] gmail.com - Correspondence added

I can't find any fault related to this in visualize_c3.pl itself. This appears to be related to the usage of "use base" on all of these classes in a single linear script. The @ISA of the class itself is not getting set up correctly, hence the rest of the issues. Replacing "use base 'foo'; use base 'bar';" with "our @ISA = qw/foo bar/;" throughout resolves the @ISA issue and therefore generates good graphs.

Tue Sep 19 23:56:01 2006 The RT System itself - Status changed from 'new' to 'open'

Tue Sep 19 23:56:17 2006 blblack [...] gmail.com - Taken

Wed Sep 20 09:50:43 2006 ted [...] tedcarnahan.com - Correspondence added

From:

ted [...] tedcarnahan.com

On Tue Sep 19 23:56:00 2006, BLBLACK wrote: Show quoted text

> > I can't find any fault related to this in visualize_c3.pl itself. This > appears to be related to the usage of "use base" on all of these classes > in a single linear script. The @ISA of the class itself is not getting > set up correctly, hence the rest of the issues. > > Replacing "use base 'foo'; use base 'bar';" with "our @ISA = qw/foo > bar/;" throughout resolves the @ISA issue and therefore generates good > graphs.

If the packages in question are in separate files, wouldn't we expect to be able to use "use base"?

Wed Sep 20 10:50:34 2006 blblack [...] gmail.com - Correspondence added

From:

BLBLACK [...] cpan.org

On Wed Sep 20 09:50:43 2006, tedcarnahan wrote: Show quoted text

> > If the packages in question are in separate files, wouldn't we expect to > be able to use "use base"? >

The issue with the original test script is that for whatever strange reason (I suspect related to the usage of "use base" in that script) @ISA for the package in question wasn't being set as expected. I would expect that it would work fine in seperate files. If you can reproduce the problem and show that @ISA for the package in question actually differs at runtime from the green lines on the graph, I'll look into it.

Wed Sep 20 13:26:57 2006 ted [...] tedcarnahan.com - Correspondence added

From:

ted [...] tedcarnahan.com

On Wed Sep 20 10:50:34 2006, BLBLACK wrote: Show quoted text

> I would expect that it would work fine in seperate files. If you can > reproduce the problem and show that @ISA for the package in question > actually differs at runtime from the green lines on the graph, I'll look > into it. >

Attached is the (almost) the same testcase I provided on the bug I filed recently on Algorithm::C3. This time the recog.pl file prints @ISA for Fixture::HasSubmission. I realize that this doesn't show that the package @ISA and the graph aren't matching, but this shows that @ISA for HasSubmission doesn't have both of the packages referred to in "use base". Perhaps we're still doing something wrong by using "use base"? Thanks, Ted

Download multiple_files.zip
application/zip 6.1k

Message body not shown because it is not plain text.

Fri Sep 22 14:23:03 2006 blblack [...] gmail.com - Correspondence added

I've finally nailed this one down. This is apparently a feature of base.pm. For each "use base" argument processed by base.pm, it first checks if the calling class ->isa the argument. If it already ->isa, then the argument doesn't get put into @ISA. So in this example: package X; use base qw/Foo/; package Y; use base qw/X Foo/; @Y::ISA will contain only "X". Foo will not be pushed onto Y's @ISA, because Y already isa X which already isa Foo. I hadn't noticed this behavior before, but its clearly completely intentional. I can only imagine what sort of havoc it could wreak on people that remove things from @ISA at runtime in a complex MI heirarchy. I *suspect* this doesn't actually affect the results of the C3 algorithm (should give the same MRO regardless of the usage of "require ...; @ISA = ..." or "use base ..."), but I could be wrong. If it turns out that this does affect the decisions of the C3 algorithm, you might need to fall back on manually doing the require/@ISA instead of using "use base", as the magic that's hacking things up is contained in base.pm.

Mon Sep 25 11:04:42 2006 ted [...] tedcarnahan.com - Correspondence added

From:

ted [...] tedcarnahan.com

Show quoted text

> I *suspect* this doesn't actually affect the results of the C3 algorithm > (should give the same MRO regardless of the usage of "require ...; @ISA > = ..." or "use base ..."), but I could be wrong. If it turns out that > this does affect the decisions of the C3 algorithm, you might need to > fall back on manually doing the require/@ISA instead of using "use > base", as the magic that's hacking things up is contained in base.pm.

I don't think that this is affecting the results of C3 - we would have seen problems in our use cases, as if the heirarchy is even slightly munged we're completely (and noticeably) hosed. We'll probably just continue to use base.pm and take the results out of visualize_c3.pl with a grain of salt. Thanks for all of the time and effort you spent looking into this. - Ted

Thu Oct 12 10:43:02 2006 blblack [...] gmail.com - Correspondence added

closing, as there's nothing we can do about this in Class::C3. If it's ever proven via testcase that the C3 MRO is mangled by base.pm's @ISA antics, we'll have to patch the docs and tell people not to use base.pm.

Thu Oct 12 10:43:04 2006 blblack [...] gmail.com - Status changed from 'open' to 'resolved'