RFC: Reduce closure magic

[Bob Rogers]

To my surprise, I found a 'ctx' member in struct Parrot_sub. It
appears that this is only used for the "autoclose" feature, which AFAICS
is not documented. The only place it is mentioned is in four
t/op/lexicals.t cases (though it is also used in three t/pmc/exception.t
cases; see the patch). The 'package-scoped closure 6 - autoclose' test
(the next-to-last one in t/op/lexicals.t) implements this Perl 6
snippet:

sub f ($x) {
sub g () { print "$x\n" };
}
f(10);
g();

in the following curious way:

.sub '&f'
.param pmc x
.lex '$x', x
.end

.sub '&g' :outer('&f')
.local pmc x
x = find_lex '$x'
print x
print "\n"
.end

.sub '&main' :main :anon
'&f'(10)
'&g'()
.end

Note that &f doesn't explicitly create a closure. Through the magic of
"autoclose," &g becomes a valid closure merely by invoking &f, and &main
prints "10" as a result.

The very fact of having a 'ctx' member in struct Parrot_sub bothers
me, but I haven't figured out how to make it fail yet, so my case
against it is purely esthetic. It seems cleaner to create the closure
and define the sub explicitly:

.sub '&f'
.param pmc x
.lex '$x', x
.local pmc sub_g, closure_g
.const .Sub sub_g = "raw_&g"
closure_g = newclosure sub_g
store_global '&g', closure_g
.end

.sub 'raw_&g' :outer('&f')
.local pmc x
x = find_lex '$x'
print x
print "\n"
.end

This also seems more natural for compilers to generate, especially when
presented with a variation like this [1]:

sub f ($x) {
if $x > $too_big {
sub g () { print "$x is too big\n" };
}
else {
sub g () { print "$x\n" };
}
}

The explicit solution also leaves &g undefined if no "sub g () { ... }"
statement is ever executed.

So it looks like "autoclose" is a convenience for writing PIR by
hand, whereas I personally think it is cleaner to create closures
explicitly. The attached patch changes all the affected test cases to
do so, and disables the feature. Is there anyone who would like to
argue that I shouldn't apply it? And if so, would you be willing to
document it?

-- Bob Rogers
http://rgrjr.dyndns.org/

[1] But I wouldn't swear that this is valid Perl 6 -- syntactically or
semantically.

[Leopold Toetsch]

Am Samstag, 7. Oktober 2006 04:07 schrieb Bob Rogers:
>    To my surprise, I found a 'ctx' member in struct Parrot_sub.  It
> appears that this is only used for the "autoclose" feature, which AFAICS
> is not documented.

Well, audreyt wanted to have this feature. But you are right: it's a bad
thing, not so much for it's magic, but becaus of the ctx member in
Parrot_sub. The Sub PMC ought to be read/only and sharable across threads and
ctx violates this.
BTW namespace_stash is another wart, we should get rid off.

leo

[Bob Rogers]

From: Leopold Toetsch <l...@toetsch.at>
Date: Sat, 7 Oct 2006 13:49:10 +0200

Am Samstag, 7. Oktober 2006 04:07 schrieb Bob Rogers:
>    To my surprise, I found a 'ctx' member in struct Parrot_sub.  It
> appears that this is only used for the "autoclose" feature, which AFAICS
> is not documented.

Well, audreyt wanted to have this feature. But you are right: it's a bad
thing, not so much for it's magic, but becaus of the ctx member in
Parrot_sub. The Sub PMC ought to be read/only and sharable across threads and
ctx violates this.

Sounds good to me. But in that case, outer_ctx is not much better
. . . but that's probably a much bigger job.

In any case, I discovered that Lua fails ~300 test cases with this
patch. So, if we still consider it worth dropping this feature, it
might be better to give it a period of deprecation before we break it
permanently. How does this sound?

-- Bob

[Larry Wall]

Note that Perl 6 requires mutable subs, at least to the extent that
you can .wrap them in place to do AOP and DBC and such. If Parrot has
immutable subs that's okay, but it forces a level of indirection on us,
and perhaps a level of non-interoperability.

Larry

[Leopold Toetsch]

Am Sonntag, 8. Oktober 2006 03:52 schrieb Bob Rogers:
> Sounds good to me.  But in that case, outer_ctx is not much better
> . . . but that's probably a much bigger job.
>
>    In any case, I discovered that Lua fails ~300 test cases with this
> patch.  So, if we still consider it worth dropping this feature, it
> might be better to give it a period of deprecation before we break it
> permanently.  How does this sound?

Hmm. Let's trying to keep autoclose then maybe. Given that I'm trying to
propose a different strategy:

We have some structures (or PMCs) which ought to be sharable across threads.
These are mainly PBC-related things, like PF structures and Subs. OTOH some
of these have interpreter (aka thread) specific stuff in them, as the
mentioned ctx's.

Instead of storing the thread-specific item directly, we store a key for that
item. The key is the same for all threads. The data associated with the key
are stored in the interpreter.

This sounds exactly like pthread_{g,s}etspecific (3p) and related, and indeed,
when pthread is available we just could use it. OTOH we could provide a more
general solution usable for all platforms.

We have a per-interpreter ResizablePMCArray (1) with the items. During
initialization we simulate pthread_once(3p) to acquire the next key, which is
the next avaiable slot in that Array (starting from 1).

(1) As stored items are currently a mixture of PMCs and other pointers, this
array would not be marked during DOD. If there's some PMC stored into it,
which isn't anchored elsewhere, it would need marking through
dod_egister_pmc(). The array would be more a RVA (*Voidp*).

leo

[Bob Rogers]

From: Leopold Toetsch <l...@toetsch.at>
Date: Sun, 8 Oct 2006 13:14:22 +0200

Am Sonntag, 8. Oktober 2006 03:52 schrieb Bob Rogers:
> Sounds good to me.  But in that case, outer_ctx is not much better
> . . . but that's probably a much bigger job.
>
>    In any case, I discovered that Lua fails ~300 test cases with this
> patch.  So, if we still consider it worth dropping this feature, it
> might be better to give it a period of deprecation before we break it
> permanently.  How does this sound?

Hmm. Let's trying to keep autoclose then maybe. Given that I'm trying to

propose a different strategy . . .

It seems to me that the original issue was that the Sub PMC, which ought
to be "pure" in some sense, was pointing to "impure" runtime data.
Having the sub point indirectly to impure data doesn't seem like an
improvement.

And, unless care is taken to allocate keys globally, a given outer
sub can only point (directly or indirectly) to one context at a time.
So code that wants to be thread-safe in the Parrot sense must create
closures explicitly anyway.

(1) As stored items are currently a mixture of PMCs and other pointers, this
array would not be marked during DOD. If there's some PMC stored into it,
which isn't anchored elsewhere, it would need marking through
dod_egister_pmc(). The array would be more a RVA (*Voidp*).

leo

Is it really worth all of this?

-- Bob