On Wed, Jul 09, 2008 at 01:27:29AM -0400, Bob Rogers wrote:
> What "foo" should do is create a closure and call that:
>
> .const .Sub inner = 'inner'
> inner = newclosure inner
> inner()
If I understand what you're saying, and then take it to what I see
as its ultimate conclusion, you're basically claiming that every
call to a subroutine involving lexicals requires a newclosure op.
Consider:
{
my $x = 1;
sub foo() { say $x; }
foo();
}
In effect this is little different from having 'foo' as an immediate
block. If we now say that a newclosure op is required to invoke 'foo',
then that means that the 'foo()' HLL subroutine has to be generated as:
.local pmc $P0
$P0 = find_name_not_null 'foo'
$P0 = newclosure $P0
$P0()
Of course, for any given subroutine call like 'foo()', the compiler
tools can't be certain at compile time that 'foo' is in fact a
Closure PMC, so we really have to generate:
$P0 = find_name_not_null 'foo'
$I0 = isa $P0, 'Closure'
unless $I0 goto xyz
$P0 = newclosure $P0
xyz:
$P0()
That 'isa' step presents a bad design smell to me -- why should we
have to test every sub for Closure-ness prior to invoking it?
It all gets worse if I have something like:
.sub 'abc' :multi('Integer')
...
.end
.sub 'abc' :multi('String') :outer('xyz')
...
.end
One of these is a Sub PMC, the other is a Closure PMC, and the thing
that is stored under symbol 'abc' is neither -- that's a MultiSub PMC.
I'm fairly certain I can't do "newclosure" on the MultiSub PMC. In
fact, the caller doesn't have any way of knowing which sub is going
to be invoked without actually invoking it, so we really can't do a
'newclosure' on it.
Ultimately I think that Closure PMCs need to be smart enough to take
a newclosure if they need one at the time they're invoked, rather than
trying to place that burden on the caller. (Note that I'm not arguing
that the current implementation is correct -- I'm just describing what
I think a correct implementation will need to do.) I also think we
probably still need to have a newclosure opcode available to allow
PIR to explicitly say when to take a closure, but I don't think it
should be required in the PIR generated for every HLL subroutine
invocation.
> [...]
>
> It may be possible to rescue Patrick's desired behavior by keeping
> track of whether the outer_sub was initialized by newclosure or not.
My 'desired behavior' is simply that we have something that works. :-)
But I suspect that this means that closure PMCs will have to keep
track of whether they've already been newclosured.
> Personally, I'd prefer if it were always an error to call a closure sub
> directly; implicitly initializing the outer context is bad enough, but
> re-initializing it is worse. (But we've had this discussion before [1],
> and I was not persuasive.)
Given the examples I listed above, I think we _have_ to provide a way for
Closure PMCs to be invoked directly from PIR, and then either the Closure PMC
vtable_invoke method or IMCC performs whatever context initialization
and/or fixups that are needed to have everything work.
(Also, a reminder that whatever we do to fix this ticket has to keep
RT #56184 working as well. :-)
Pm
