Re: Patches for wip-rtl

[Noah Lavine <noah.b.lavine@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

Hello,

On Mon, Apr 22, 2013 at 4:27 PM, Andy Wingo <wingo@pobox.com> wrote:

> Hi Noah,
>
> Do I understand you correctly that you're just making a sequence of
> non-idiomatic instructions because you know that doing the
> assert-nargs-ee will check nargs?  I would think in a normal case you'd
> want to do "assert-nargs-ge 0", then "bind-rest 0", then "reserve-locals
> 0" (or not; in this case it would not be necessary).
>

Yes, that's exactly why I'm doing it. I just wanted to find a way to test
that the bug was fixed and stayed fixed. In this case that's a bit weird,
because we probably don't want to guarantee that this pattern of
instructions will continue to work, but I don't have a great alternative. I
think any sort of test for this is going to be dependent on the internals
of the VM. (My earlier email about adding accessors for the VM was about
this same bug, but then I realized that nargs isn't part of the vm
structure so I couldn't solve it like that.)


> However the bug you have seen with bind-rest is probably also present
> with keyword arguments.  We should probably just change the
> reserve-locals instruction to also take "nargs" as a value, given that
> we know it statically at compile-time.
>
> Does that make sense to you?
>

I'm not sure. As long as nargs is going to be a local variable, I don't see
a reason not to use it. It might make more sense to either keep it the way
it is or change every instruction that uses nargs. But I haven't looked to
see how many there are, or how important it is that they know nargs.

Noah

[-- Attachment #2: Type: text/html, Size: 2259 bytes --]