A day of Rakudo bug-fixing

JonathanWorthington on 2008-11-27T00:34:11

Today was my weekly Vienna.pm-funded Rakudo Day, and I spent it digging into the RT queue to see what bugs and patches people had been sending in and do some fixes and patch application. Here's what I got up to.

  • bacek noted that writing "my $a; $a = $a + 1i;" led to Parrot giving us a multiple dispatch failure. I fixed this by making sure any non-Complex values were coerced before we tried to get Parrot to do the addition, so you now get the auto-vivification that you expect and this just works.
  • Continuing the theme of reviewing making stuff work when you stick it inside a namespace, Chris Dolan supplied another patch, this time taking on roles. It solved half of the problem; I provided a fix to solve the other half and applied it along with some tests. Thanks to Chris for the patch and prodding me to do the other bits that were needed.
  • bacek discovered an interesting bug when you tried to name an enum element the same way as certain Parrot vtable functions that I'd overridden. Turns out I forgot the anon flag, so they were added as normal non-vtable methods too. D'oh. And fixed.
  • Ilya reported that trying to declare a Range type constraint on an attribute in a class led to a Null PMC Access. I was a tad baffled, until I discovered a missing null check in an attribute cloning routine.

  • A ticket form masak expected the eval with a block would work. The spec says otherwise, but we were crashing in a bizarre way when people tried to use the block form - on exit! I put in a type check on the parameter to eval, as shown in S29, so it tells you outright that it's wrong now.
  • Fixing this reminded me that the error message for a type check failure could be more helpful and mention which sub you had invoked when the failure happened. So I made it so.
  • masak wished that '$a .= "A"' would give you a more helpful error message. In fact, the compiler was trying to, but was calling panic on something that didn't know how to. Fixed and now it tells you that you were meant to write a method name over on the RHS.
  • masak also reported that .= didn't work out so well if the LHS was an index into an array (like @a[0]). It didn't work for doing the equivalent with hashes either. Thanks to PAST improvements since I did the first cut of .=, it was now trivial to refactor .= to do the right thing.
  • Patrick and I spent a while discussing a couple of the messier areas of actions.pm and worked out ways to improve them. There's tasks for both of us in there. We also talked about how to implement context variables, and I think Patrick plans to hack on that soonish. Those will be cool to have. :-)
  • Fixed a bug reported by masak relating to lexicals with the & sigil, and added a couple of spectests to cover it.
  • Moritz pointed to an integration test that he'd added, known as the man or boy test. Rakudo turned out to be a bit of a boy, but a couple of fixes later it's now a man. (Fixes were to make parameters with the & sigil work properly - they were meant to but there was an small bug - and fixing up 'is copy' a bit. We run through the first 10 values, and then hit Parrot's developer-aiding recursion limit in the 11th. But computing the first 10 pretty much shows we can do it. I pondered upping the limit for Rakudo (a default value of 1000 calls deep is compiled into Parrot, but it's changeable at runtime), but the limit is good for now while we're in development. Better to catch infinite recursions early.
  • Ilya discovered a bug that was introduced when we refactored containers. It showed up when you passed an array and then iterated over it. I put in something that I thought would fix it (it fixed every test case in the email, all of spectest that we were passing before and some tests from the email that Moritz had put in a test file). However, then Patrick pointed out a case that it didn't work for. No good solution yet, but I added his test case to the spectests so we don't forget to address it. In the meantime, hopefully this makes things a little better for Ilya.
  • Responded to and/or commented on a few other tickets.

So, a day full of bug fixing rather than doing shiny new features. Which I guess makes not so exciting reading, but I hope this contributes to a more stable Rakudo and a better experience using it. Plus there's tests to cover the issues that were reported and fixed, meaning we've got better test coverage and some hope of not regressing on any of these in the future. :-) Thanks to everyone who provided the feedback so we knew these bugs existed, and to Vienna.pm for funding me to do the fixes.


man or boy

karavelov on 2008-11-27T04:16:57

I have not found Moritz's code for the man or boy test. My try with the last revision goes to 9. It reaches max recursion depth for 10. Here it is:

sub A($k is copy, $x1, $x2, $x3, $x4, $x5) {
my $B = sub { A(--$k, $B, $x1, $x2, $x3, $x4) };
$k <= 0 ?? $x4() + $x5() !! $B();
}
say A(9, sub{1}, sub{-1}, sub{-1}, sub{1}, sub{0} );

What is the need for & sigil you mention? I have done by hand some crude tailcall optimization in the generated PIR and it reaches the recursion depth limit on 13... go figure.

Re:man or boy

infinoid on 2008-11-27T15:19:21

I have not found Moritz's code for the man or boy test.

Moritz's test is here: http://svn.pugscode.org/pugs/t/spec/integration/man-or-boy.t

Re:man or boy

JonathanWorthington on 2008-11-28T02:01:16

You don't have to use the & sigil, but if you do (e.g. sub foo (&x) { ... }) then you can call the sub just as x() within the sub foo.

It's not surprising that implementing the tailcall optimization helped. I suspect we need to be a tad careful when choosing to emit such things in the compiler, since it may screw up the CALLER pseudo-package...not thought too much about the full implications of this for optimization yet though and it may or may not turn out to matter...