On Mon, Jun 23, 2008 at 06:27:31PM -0500, Patrick R. Michaud wrote:
> There appears to be a fundamental design problem in Parrot's
> current implementation of :outer.  The short summary is that
> :outer("sub_name") doesn't provide sufficient specificity
> to accurately resolve an outer sub.
> [...]
> Yet another approach would be to keep things as they are now,
> but have :outer only refer to the closest (most recent) version
> of a sub with that name.  We still may have to be careful about
> dealing with :multi subs, though, and it might be possible to craft
> some HLL code where it's not possible to make this approach work.

The attached patch implements this last approach, causing
:outer('xyz') to refer to the most recently compiled sub
named 'xyz', as opposed to always using the first one.  This
works as long as no other sub with the name 'xyz' is
interposed between the true outer 'xyz' and the sub
using the :outer('xyz'), which should be the case
for most things compiled with PCT.  (I'm certain it's
possible to create a PAST structure that violates this,
but we can potentially fix that if/when it occurs.)

The patch to the test file verifies that two identical
:outer flags each refer to the appropriate sub.

I don't think this completely resolves the lexical issues,
and it may hide the issue in RT#56184 (which I'll elaborate
on later).  But it may be a useful approach for now.

I'm looking for review/comments on this patch before applying it.

Thanks!

Pm
Index: compilers/imcc/pbc.c
===================================================================
--- compilers/imcc/pbc.c        (revision 28672)
+++ compilers/imcc/pbc.c        (working copy)
@@ -944,6 +944,7 @@
     size_t  len;
     PMC    *current;
     STRING *cur_name;
+    PMC    *sub_pmc = 0;
 
     if (!unit->outer)
         return NULL;
@@ -961,12 +962,13 @@
 
     for (s = globals.cs->first; s; s = s->next) {
         SymReg * const sub = s->unit->instructions->symregs[0];
-
-        if (STREQ(sub->name, unit->outer->name)) {
-            PObj_get_FLAGS(s->unit->sub_pmc) |= SUB_FLAG_IS_OUTER;
-            return s->unit->sub_pmc;
-        }
+        if (STREQ(sub->name, unit->outer->name))
+            sub_pmc = s->unit->sub_pmc;
     }
+    if (sub_pmc) {
+        PObj_get_FLAGS(sub_pmc) |= SUB_FLAG_IS_OUTER;
+        return sub_pmc;
+    }
 
     /* could be eval too; check if :outer is the current sub */
     current = CONTEXT(interp)->current_sub;
Index: t/pmc/sub.t
===================================================================
--- t/pmc/sub.t (revision 28672)
+++ t/pmc/sub.t (working copy)
@@ -7,7 +7,7 @@
 use lib qw( . lib ../lib ../../lib );
 
 use Test::More;
-use Parrot::Test tests => 63;
+use Parrot::Test tests => 64;
 use Parrot::Config;
 
 =head1 NAME
@@ -1469,6 +1469,52 @@
 I can has outer?
 OUTPUT
 
+pir_output_is( <<'CODE', <<'OUTPUT', ':outer with identical sub names' );
+.sub 'main' :main
+    $P0 = get_hll_global ['ABC'], 'outer'
+    $P0('ABC lex')
+
+    $P0 = get_hll_global ['DEF'], 'outer'
+    $P0('DEF lex')
+.end
+
+.namespace ['ABC']
+.sub 'outer'
+    .param pmc x
+    .lex '$abc', x
+    say 'ABC::outer'
+    'inner'()
+.end
+
+.sub 'inner' :outer('outer')
+    say 'ABC::inner'
+    $P0 = find_lex '$abc'
+    say $P0
+.end
+
+.namespace ['DEF']
+.sub 'outer'
+    .param pmc x
+    .lex '$def', x
+    say 'DEF::outer'
+    'inner'()
+.end
+
+.sub 'inner' :outer('outer')
+    say 'DEF::inner'
+    $P0 = find_lex '$def'
+    say $P0
+.end
+CODE
+ABC::outer
+ABC::inner
+ABC lex
+DEF::outer
+DEF::inner
+DEF lex
+OUTPUT
+
+
 # Local Variables:
 #   mode: cperl
 #   cperl-indent-level: 4

Reply via email to