Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible mro bug/change between 5.8.8 and 5.10.0 #9746

Closed
p5pRT opened this issue May 28, 2009 · 18 comments
Closed

Possible mro bug/change between 5.8.8 and 5.10.0 #9746

p5pRT opened this issue May 28, 2009 · 18 comments

Comments

@p5pRT
Copy link

p5pRT commented May 28, 2009

Migrated from rt.perl.org#66096 (status was 'resolved')

Searchable as RT66096$

@p5pRT
Copy link
Author

p5pRT commented May 28, 2009

From @nwc10

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was talking about
it with mst last night and discovered the mro resolution order is different.

I don't know much about mro or C3, but Matt thinks it might be a 5.10
bug/change. For details, see...
http​://rt.cpan.org/Public/Bug/Display.html?id=40343

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

I'm currently trying to create a small test case based on this bug
report. (Triggering this bug requires 47 modules...)

(My very first guess however goes to MRO​::Compat)

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

p5p@spam.wizbit.be - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

Running it​:

5.008008
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::Two

5.010000
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::One

5.8.8 calls RT66096​::Two while 5.10.0 calls RT66096​::One...

mk_group_accessors is called via RT66096​::Test->mk_group_accessors
@​RT66096​::Test​::ISA = RT66096​::A3
@​RT66096​::A3​::ISA = RT66096
RT66096​::A3 calls load_components A2

@​RT6609​::A2​::ISA = RT66096
RT66096​::A2 calls load_components A1 Two

@​RT66096​::A1​::ISA = RT6096​::One

@​RT6609​::A2​::ISA is updated to RT66096​::A1, RT66096​::Two, RT66096
@​RT6609​::A3​::ISA is updated to RT66096​::A2, RT66096

@​RT66096​::A1​::ISA is set to RT6096​::One => so @​RT6609​::A3​::ISA really
is (or should be) equal to 'RT66096​::One, RT66096​::Two, RT66096'

But in perl-5.8.8 the calls goes to RT66096​::Two and in perl-5.10.0 to
RT66096​::One. So which one is right?

I believe the behaviour in perl-5.10.0 is 'correct' and the behaviour
in perl-5.8.8 is 'wrong'

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From [Unknown Contact. See original ticket]

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

Running it​:

5.008008
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::Two

5.010000
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::One

5.8.8 calls RT66096​::Two while 5.10.0 calls RT66096​::One...

mk_group_accessors is called via RT66096​::Test->mk_group_accessors
@​RT66096​::Test​::ISA = RT66096​::A3
@​RT66096​::A3​::ISA = RT66096
RT66096​::A3 calls load_components A2

@​RT6609​::A2​::ISA = RT66096
RT66096​::A2 calls load_components A1 Two

@​RT66096​::A1​::ISA = RT6096​::One

@​RT6609​::A2​::ISA is updated to RT66096​::A1, RT66096​::Two, RT66096
@​RT6609​::A3​::ISA is updated to RT66096​::A2, RT66096

@​RT66096​::A1​::ISA is set to RT6096​::One => so @​RT6609​::A3​::ISA really
is (or should be) equal to 'RT66096​::One, RT66096​::Two, RT66096'

But in perl-5.8.8 the calls goes to RT66096​::Two and in perl-5.10.0 to
RT66096​::One. So which one is right?

I believe the behaviour in perl-5.10.0 is 'correct' and the behaviour
in perl-5.8.8 is 'wrong'

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

On Mon Jun 01 11​:23​:48 2009, animator wrote​:

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.

It looks like RT dropped my attachment for some reason... (rt-66096-
new.t) so I'm retrying with rt-66096-new.pl.

It (still) requires Class​::C3 and MRO​::Compat

Running it​:

5.008008
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::Two

5.010000
Updated @​ISA for RT66096​::A2 => RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A2 => RT66096​::A1, RT66096​::Two, RT66096
Updated @​ISA for RT66096​::A3 => RT66096​::A2, RT66096
Calling mk_group_accessors in RT66096​::One

5.8.8 calls RT66096​::Two while 5.10.0 calls RT66096​::One...

mk_group_accessors is called via RT66096​::Test->mk_group_accessors
@​RT66096​::Test​::ISA = RT66096​::A3
@​RT66096​::A3​::ISA = RT66096
RT66096​::A3 calls load_components A2

@​RT6609​::A2​::ISA = RT66096
RT66096​::A2 calls load_components A1 Two

@​RT66096​::A1​::ISA = RT6096​::One

@​RT6609​::A2​::ISA is updated to RT66096​::A1, RT66096​::Two, RT66096
@​RT6609​::A3​::ISA is updated to RT66096​::A2, RT66096

@​RT66096​::A1​::ISA is set to RT6096​::One => so @​RT6609​::A3​::ISA really
is (or should be) equal to 'RT66096​::One, RT66096​::Two, RT66096'

But in perl-5.8.8 the calls goes to RT66096​::Two and in perl-5.10.0
to
RT66096​::One. So which one is right?

I believe the behaviour in perl-5.10.0 is 'correct' and the behaviour
in perl-5.8.8 is 'wrong'

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

rt-66096-new.pl

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

On Mon Jun 01 11​:23​:48 2009, animator wrote​:

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

Trimmed it some more..

[snip]

I believe the behaviour in perl-5.10.0 is 'correct' and the behaviour
in perl-5.8.8 is 'wrong'

(After thinking about it some more and reading the Class​::C3
documentation​:)

Looking at the inherintance tree​:

  RT66096​::One
  / \
RT66096​::A1 RT66096​::Two
  \ /
  RT66096​::A2
  |
  RT66096​::Test

When the mro is set to Class​::C3 then it should never call
RT66096​::One​::mk_group_accessors before checking/calling
RT66096​::Two​::mk_group_accessors and/or RT66096​::A1.

When the attached script is run as is then it outputs​:

5.008008
Current mro​: dfs
New mro​: dfs
Calling mk_group_accessors in RT66096​::Two

5.010000
Current mro​: dfs
New mro​: dfs
Calling mk_group_accessors in RT66096​::One

If the commented line is enabled​:

5.008008
Current mro​: dfs
New mro​: c3
Calling mk_group_accessors in RT66096​::Two

5.010000
Current mro​: dfs
New mro​: c3
Calling mk_group_accessors in RT66096​::Two

Summary​:

In perl-5.10.0 mro​::set_mro(__PACKAGE__, 'c3') is needed in
RT66096​::Test while it is not needed for perl-5.8.8.

Unanswered question​: what is the correct/expected behaviour?

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

rt-66096-2.pl

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From [Unknown Contact. See original ticket]

On Mon Jun 01 11​:23​:48 2009, animator wrote​:

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

Trimmed it some more..

[snip]

I believe the behaviour in perl-5.10.0 is 'correct' and the behaviour
in perl-5.8.8 is 'wrong'

(After thinking about it some more and reading the Class​::C3
documentation​:)

Looking at the inherintance tree​:

  RT66096​::One
  / \
RT66096​::A1 RT66096​::Two
  \ /
  RT66096​::A2
  |
  RT66096​::Test

When the mro is set to Class​::C3 then it should never call
RT66096​::One​::mk_group_accessors before checking/calling
RT66096​::Two​::mk_group_accessors and/or RT66096​::A1.

When the attached script is run as is then it outputs​:

5.008008
Current mro​: dfs
New mro​: dfs
Calling mk_group_accessors in RT66096​::Two

5.010000
Current mro​: dfs
New mro​: dfs
Calling mk_group_accessors in RT66096​::One

If the commented line is enabled​:

5.008008
Current mro​: dfs
New mro​: c3
Calling mk_group_accessors in RT66096​::Two

5.010000
Current mro​: dfs
New mro​: c3
Calling mk_group_accessors in RT66096​::Two

Summary​:

In perl-5.10.0 mro​::set_mro(__PACKAGE__, 'c3') is needed in
RT66096​::Test while it is not needed for perl-5.8.8.

Unanswered question​: what is the correct/expected behaviour?

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

On Mon Jun 01 13​:04​:26 2009, animator wrote​:

On Mon Jun 01 11​:23​:48 2009, animator wrote​:

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

[snip]

Looking at the inherintance tree​:

        RT66096​::One
       /           \\

RT66096​::A1 RT66096​::Two
\ /
RT66096​::A2
|
RT66096​::Test

[snip]

Replying on myself again...

- RT66096​::One defines the method mk_group_accessors
- RT66096​::Two defines the method mk_group_accessors

- mro of RT66096​::A2 to c3,
- mro of RT66096​::Test is set to dfs

What I believe is happening​:

When the MRO​::Compat is used (meaning on perl < 5.9.5) then Class​::C3
creates a method
*RT66096​::A2​::mk_group_accessors which calls
RT66096​::Two​::mk_group_accessors.

When the MRO​::Compat code is not used (meaning on perl >= 5.9.5) it
checks the mro type of RT66096​::Test.
It will eventually go to mro.c -> S_mro_get_linear_isa_dfs since the
mro type of RT66096​::Test is set to dfs.
Inside S_mro_get_linear_isa_dfs it does not appear to check the mro
type of the classes it inherits from. Meaning​: it assumes dfs and not
c3.
The same applies for S_mro_get_linear_isa_c3.

In the attached test script there are two scenarions​: or the lines
commented with # TEST1 or the lines commented with # TEST2​:

With #TEST1​:
5.008008
RT66096​::Two​::mk_group_accessors called

5.010000
RT66096​::One​::mk_group_accessors called

With #TEST2​:
5.008008
RT66096​::One​::mk_group_accessors called

5.010000
RT66096​::Two​::mk_group_accessors called

So the mismatch is​:

On perl < 5.9.5 it sort of 'checks' the mro type of RT66096​::Test and
RT66096​::A2 when calling RT66096->mk_group_accessors.
On perl >= 5.9.5 it only 'checks' the mro type of RT66096​::Test when
calling RT66096->mk_group_accessors.

(mro.c has always behaved like this)

This means that either MRO​::Compat/Class​::C3 has to change or mro.c has
to change...

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From p5p@spam.wizbit.be

rt-66096-4.pl

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2009

From [Unknown Contact. See original ticket]

On Mon Jun 01 13​:04​:26 2009, animator wrote​:

On Mon Jun 01 11​:23​:48 2009, animator wrote​:

Schwern mailed p5p in 490251F1.30507@​pobox.com
http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-
10/msg00675.html

To quote​:

There's a problem with DBIx​::Class​::CDBICompat on 5.10.0. I was
talking about
it with mst last night and discovered the mro resolution order is
different.

Trimmed down version attached.
It (still) requires Class​::C3 and MRO​::Compat

[snip]

Looking at the inherintance tree​:

        RT66096&#8203;::One
       /           \\

RT66096​::A1 RT66096​::Two
\ /
RT66096​::A2
|
RT66096​::Test

[snip]

Replying on myself again...

- RT66096​::One defines the method mk_group_accessors
- RT66096​::Two defines the method mk_group_accessors

- mro of RT66096​::A2 to c3,
- mro of RT66096​::Test is set to dfs

What I believe is happening​:

When the MRO​::Compat is used (meaning on perl < 5.9.5) then Class​::C3
creates a method
*RT66096​::A2​::mk_group_accessors which calls
RT66096​::Two​::mk_group_accessors.

When the MRO​::Compat code is not used (meaning on perl >= 5.9.5) it
checks the mro type of RT66096​::Test.
It will eventually go to mro.c -> S_mro_get_linear_isa_dfs since the
mro type of RT66096​::Test is set to dfs.
Inside S_mro_get_linear_isa_dfs it does not appear to check the mro
type of the classes it inherits from. Meaning​: it assumes dfs and not
c3.
The same applies for S_mro_get_linear_isa_c3.

In the attached test script there are two scenarions​: or the lines
commented with # TEST1 or the lines commented with # TEST2​:

With #TEST1​:
5.008008
RT66096​::Two​::mk_group_accessors called

5.010000
RT66096​::One​::mk_group_accessors called

With #TEST2​:
5.008008
RT66096​::One​::mk_group_accessors called

5.010000
RT66096​::Two​::mk_group_accessors called

So the mismatch is​:

On perl < 5.9.5 it sort of 'checks' the mro type of RT66096​::Test and
RT66096​::A2 when calling RT66096->mk_group_accessors.
On perl >= 5.9.5 it only 'checks' the mro type of RT66096​::Test when
calling RT66096->mk_group_accessors.

(mro.c has always behaved like this)

This means that either MRO​::Compat/Class​::C3 has to change or mro.c has
to change...

Best regards,

Bram

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2009

From blblack@gmail.com

I've trimmed down the test-case script above even further (see
attached), and AFAICS 5.10's behavior is correct. It is
MRO​::Compat/Class​::C3 on 5.8.8 (and presumably anything older) which are
exhibiting incorrect behavior.

Correct behavior, as exhibited by 5.10, is that when calling a method on
a given class "A", only the mro type (c3 vs dfs) of class "A" matters,
not the mro types of any superclasses. This is the output of the
newly-trimmed test-case script for me​:

ika​:c3bug blblack$ perl rt-66096-5.pl ; ~/P/bin/perl rt-66096-5.pl
5.008009​:
resolved to RT66096​::Two, correct answer is RT66096​::Base
resolved to RT66096​::Two, correct answer is RT66096​::Two
5.010000​:
resolved to RT66096​::Base, correct answer is RT66096​::Base
resolved to RT66096​::Two, correct answer is RT66096​::Two

This needs to be solved over on CPAN in Class​::C3 and/or various related
modules, but it's not a bug for 5.10.x and this bug should be closed
here, IMHO.

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2009

From blblack@gmail.com

rt-66096-5.pl

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2009

From @nwc10

Given that Brandon diagnosed this difference of behaviour between 5.10's
code and and CPAN's code as "5.10 is correct, CPAN code needs fixing",
I believe we can mark this core bug as resolved.

Sadly we don't (yet) have the facility to migrate bugs between
rt.perl.org and rt.cpan.org - how long until sd can do this?

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2009

From [Unknown Contact. See original ticket]

Given that Brandon diagnosed this difference of behaviour between 5.10's
code and and CPAN's code as "5.10 is correct, CPAN code needs fixing",
I believe we can mark this core bug as resolved.

Sadly we don't (yet) have the facility to migrate bugs between
rt.perl.org and rt.cpan.org - how long until sd can do this?

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2009

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant