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

.clone not doing the right kinda thing with arrays in Rakudo #2737

Closed
p6rt opened this issue Apr 28, 2012 · 9 comments
Closed

.clone not doing the right kinda thing with arrays in Rakudo #2737

p6rt opened this issue Apr 28, 2012 · 9 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Apr 28, 2012

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

Searchable as RT112666$

@p6rt
Copy link
Author

p6rt commented Apr 28, 2012

From @masak

<gfldex> i may have found another bug and you wont like that one :)
<gfldex> r​: https://gist.github.com/2519081
<p6eval> rakudo 2a962e​: OUTPUT«0␤App​::exit called␤App​::exit called␤»
<gfldex> r​: https://gist.github.com/2519083
<p6eval> rakudo 2a962e​: OUTPUT«App​::exit called␤»
<gfldex> the only difference of those two gists is in line 33
<gfldex> it should call App​::exit twice for both
<masak> please golf this, and I'll submit it.
<gfldex> masak​: that's as short as i can get it https://gist.github.com/2519182
<gfldex> masak​: update​: https://gist.github.com/2519182
* masak tries to golf it further
<jnthn> .oO( and finally putts it in RT )
<masak> generally, if it contains 'use v6;', it's too long :P
<gfldex> masak​: i got it a little shorter myself
<gfldex> https://gist.github.com/2519279
<masak> still very long.
<masak> you can't see what's happening when it spans several lines like that.
<gfldex> i can put it all on one line if you like :)
<masak> and remove all the other cruft.
<gfldex> masak​: not even that, update​: https://gist.github.com/2519279
<gfldex> where the heck does it take the @​a from in the wrap({block}) ?
<jnthn> I'm not really surprised that doesn't work...
<jnthn> The @​a would be the one in the static lexpad I guess...
<jnthn> Whereas when the mainline actually runs, it gets a fresh @​a
<gfldex> so it should either work or complain that @​a is not declared?
<jnthn> But it is declared
<jnthn> You're talking about two different @​as
<jnthn> Because of BEGIN time
<jnthn> That trait_mod runs at BEGIN time.
<masak> yes, it is BEGIN time. why does that make it two different @​a ?
<jnthn> sub foo() { my @​a }; foo; foo; # how many @​a ?
<gfldex> if you add @​a.elems; right before $r.wrap it works
<jnthn> Hm
<gfldex> and @​a.elems is called at BEGIN indeed
<jnthn> Yeah...
<jnthn> That may well be a case of "works by accident"
<jnthn> You actually vivify the @​a then
<jnthn> I guess you vivify the one in the static lexpad.
<jnthn> And then it gets copied
<gfldex> but why does it get copied instead of forming a closure?
<jnthn> nom​: sub foo() { my @​a; BEGIN { @​a = 1,2,3 }; say @​a; }; foo(); foo();
<p6eval> rakudo 2a962e​: OUTPUT«elements() not implemented in class 'Mu' [...]
<jnthn> Wow.
<masak> here we go.
<masak> r​: role R { has @​.s is rw }; multi trait_mod​:<is>($r, :$x!) {
$r does R; sub h(|$){ for $r.s { &^m() } }; $r.wrap(&h) }; sub b is x
{}; push &b.s, { say "OH HAI" }; b
<p6eval> rakudo 2a962e​: ( no output )
<masak> r​: role R { has @​.s is rw }; multi trait_mod​:<is>($r, :$x!) {
$r does R; $r.s; sub h(|$){ for $r.s { &^m() } }; $r.wrap(&h) }; sub b
is x {}; push &b.s, { say "OH HAI" }; b
<p6eval> rakudo 6c9661​: OUTPUT«OH HAI␤»
<masak> note.
<masak> only difference.
<masak> '$r.s'
* masak submits rakudobug
<jnthn> :/
<masak> whatever the cause or underlying subtleties with BEGIN time,
that *shouldn't* happen.
<jnthn> OK, maybe I should find a way to make this not work at all.
<gfldex> but how do you carry state in a trait_mod​:<is> then?
<masak> jnthn​: that sounds like a good start.
*masak does another RT ticket of massive bookkeeping
<masak> gfldex++ # extending our limits :)
<gfldex> or to ask the question from a perl6user viewpoint​: how do i
do that correctly?
<gfldex> it's funny how one can find bugs in a language by actually
using that language :)
<masak> how else would you find them?
<gfldex> well, you have design by committee and then some magic
<gfldex> that's at least what managers tend to believe
<jnthn> masak​: The bug you filed isn't to do with BEGIN time.
<jnthn> It's to do with .clone not doing the right kinda thing with arrays.
<jnthn> Just worked out what's going on.
<masak> ok.
<masak> I'm still editing it, so I can still change the subject line. :)

@p6rt
Copy link
Author

p6rt commented Apr 28, 2012

From @masak

<gfldex> r​: https://gist.github.com/2519842
<p6eval> rakudo 2a962e​: OUTPUT«(signal SEGV)»
<gfldex> masak​: could you add https://gist.github.com/2519842 to RT112666
please?
<masak> gfldex​: will do.

@p6rt
Copy link
Author

p6rt commented Apr 28, 2012

@masak - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented May 30, 2012

From @diakopter

On Sat Apr 28 09​:10​:16 2012, masak wrote​:

<gfldex> r​: https://gist.github.com/2519842
<p6eval> rakudo 2a962e​: OUTPUT�(signal SEGV)�
<gfldex> masak​: could you add https://gist.github.com/2519842 to
RT112666
please?
<masak> gfldex​: will do.

None of the 3 versions of that gist segfaults any longer on the latest
rakudo. They all output App​::exit called

@p6rt
Copy link
Author

p6rt commented May 30, 2012

From @diakopter

On Tue May 29 17​:43​:29 2012, diakopter wrote​:

On Sat Apr 28 09​:10​:16 2012, masak wrote​:

<gfldex> r​: https://gist.github.com/2519842
<p6eval> rakudo 2a962e​: OUTPUT�(signal SEGV)�
<gfldex> masak​: could you add https://gist.github.com/2519842 to
RT112666
please?
<masak> gfldex​: will do.

None of the 3 versions of that gist segfaults any longer on the latest
rakudo. They all output App​::exit called

(but the original issue is still existing.

@p6rt
Copy link
Author

p6rt commented Oct 18, 2015

From @niner

13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: IIRC, it was .clone not being deep enough, but I don't remember exactly either :(
13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: Maybe not creating fresh Scalar containers or so

@p6rt
Copy link
Author

p6rt commented Nov 18, 2015

From @peschwa

On Sun Oct 18 04​:50​:28 2015, nine@​detonation.org wrote​:

13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: IIRC, it was .clone
not being deep enough, but I don't remember exactly either :(
13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: Maybe not creating
fresh Scalar containers or so

Current behavior​:

14​:17 < psch> r​: role R { has @​.s is rw }; multi trait_mod​:<is>(Routine $r, :$x!) { $r does R; sub h(|){ for $r.s { &^m() } }; $r.wrap(&h) }; sub b is x {}; push &b.s, { say "OH HAI" }; b # slightly adapted from RT #​112666
14​:17 <+camelia> rakudo-{moar,jvm} 61e505​: OUTPUT«OH HAI␤»
14​:18 < psch> changes are​: signature for &h can't have |$, and the trait signature needs the Routine type for $r
14​:18 < psch> but that's the one that didn't output anything in the ticket, except for those two changes
14​:19 * psch adds that to the ticket

Note that this is code snippet that doesn't explicitly vivify $r.s in the trait declaration, yet the output matches the one that does​:

14​:22 <psch> r​: role R { has @​.s is rw }; multi trait_mod​:<is>(Routine $r, :$x!) { $r does R; $r.s; sub h(|){ for $r.s { &^m() } }; $r.wrap(&h) }; sub b is x {}; push &b.s, { say "OH HAI" }; b
14​:22 <camelia> rakudo-{moar,jvm} 61e505​: OUTPUT«OH HAI␤»

@p6rt
Copy link
Author

p6rt commented Dec 15, 2015

From @jnthn

On Wed Nov 18 06​:24​:03 2015, peschwa@​gmail.com wrote​:

On Sun Oct 18 04​:50​:28 2015, nine@​detonation.org wrote​:

13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: IIRC, it was .clone
not being deep enough, but I don't remember exactly either :(
13​:49 <+yoleaux> 17 Oct 2015 17​:45Z <jnthn> nine​: Maybe not creating
fresh Scalar containers or so

Current behavior​:

14​:17 < psch> r​: role R { has @​.s is rw }; multi
trait_mod​:<is>(Routine $r, :$x!) { $r does R; sub h(|){ for $r.s {
&^m() } }; $r.wrap(&h) }; sub b is x {}; push &b.s, { say "OH HAI" };
b # slightly adapted from RT #​112666
14​:17 <+camelia> rakudo-{moar,jvm} 61e505​: OUTPUT«OH HAI␤»
14​:18 < psch> changes are​: signature for &h can't have |$, and the
trait signature needs the Routine type for $r
14​:18 < psch> but that's the one that didn't output anything in the
ticket, except for those two changes
14​:19 * psch adds that to the ticket

Note that this is code snippet that doesn't explicitly vivify $r.s in
the trait declaration, yet the output matches the one that does​:

14​:22 <psch> r​: role R { has @​.s is rw }; multi trait_mod​:<is>(Routine
$r, :$x!) { $r does R; $r.s; sub h(|){ for $r.s { &^m() } };
$r.wrap(&h) }; sub b is x {}; push &b.s, { say "OH HAI" }; b
14​:22 <camelia> rakudo-{moar,jvm} 61e505​: OUTPUT«OH HAI␤»

This suggested the bug was fixed. I tried the code on its own. Worked. Put it in a test file. Failed. Wait, what?

Turns out the significant difference was the whole lot being in a nested block. The original bug never went away, but an optimization hid it. And the bug itself came from an optimization involving lazy allocation of attribute storage, which we just can't rely on getting away with when doing mixins to meta-objects. So, fixed that. Test in S14-traits/routines.t.

@p6rt p6rt closed this as completed Dec 15, 2015
@p6rt
Copy link
Author

p6rt commented Dec 15, 2015

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

@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant