When you try to invoke a sub that doesn't exist, Parrot currently gives the 
unhelpful error message "Null PMC access in invoke()".  Sometimes you can 
figure out what's wrong given the backtrace.  Often you can't.

It would be nice instead to get a more specific error message indicating that 
you tried to invoke a sub that doesn't exist.

There are a couple of ways to do this.

fixup_globals() in compilers/imcc/pbc.c runs at the end of emitting a PBC 
segment.  It converts all of the symbolic (named) lookups of subs into direct 
lookups, where Parrot already knows about subs of the appropriate name.  It 
translates the remaining lookups into find_name ops.

The right approach is probably to modify this code to add a check for PMCNULL 
for the returned PMC and throw an exception in that case.  However, because 
this fixup runs at the end of the PBC process, it's really difficult to 
splice in multiple ops, especially when they need to keep around the string 
constant or register containing the symbol for dynamic lookup.

This may be fixable when we have PBC PMCs.

The attached patch -- for discussion only -- adds an experimental op which 
performs the lookup and throws an exception if the sub is PMCNULL.  Nothing 
else uses this op, and it doesn't perturb the existing find_name op, which is 
important.

If the op goes away in the future or if there's an easier way to emit this 
error message, that's fine, but I do suggest that adding this error message 
improves debuggability.

All tests still pass; this feature obviously needs tests.

-- c

=== compilers/imcc/pbc.c
==================================================================
--- compilers/imcc/pbc.c	(revision 27278)
+++ compilers/imcc/pbc.c	(local)
@@ -655,7 +655,7 @@
                     SymReg * const nam = mk_const(interp, fixup->name,
                             fixup->type & VT_ENCODED ? 'U' : 'S');
 
-                    op = interp->op_lib->op_code("find_name_p_sc", 1);
+                    op = interp->op_lib->op_code("find_sub_not_null_p_sc", 1);
                     PARROT_ASSERT(op);
 
                     interp->code->base.data[addr] = op;
=== src/ops/core.ops
==================================================================
--- src/ops/core.ops	(revision 27278)
+++ src/ops/core.ops	(local)
@@ -405,12 +405,13 @@
 =cut
 
 inline op invokecc(invar PMC) :flow {
-    opcode_t *dest;
-    PMC * const p = $1;
-    dest = expr NEXT();
+    PMC      * const p     = $1;
+    opcode_t *dest         = expr NEXT();
+
     interp->current_object = NULL;
-    interp->current_cont = NEED_CONTINUATION;
-    dest = (opcode_t *)p->vtable->invoke(interp, p, dest);
+    interp->current_cont   = NEED_CONTINUATION;
+    dest                   = (opcode_t *)p->vtable->invoke(interp, p, dest);
+
     goto ADDRESS(dest);
 }
 
=== src/ops/experimental.ops
==================================================================
--- src/ops/experimental.ops	(revision 27278)
+++ src/ops/experimental.ops	(local)
@@ -412,7 +412,18 @@
   $1 = string_substr(interp, $2, $3, $4, &dest, 1);
 }
 
+=item C<find_sub_not_null>(out PMC, in STR)
 
+inline op find_sub_not_null(out PMC, in STR) :base_core {
+    PMC *sub = Parrot_find_name_op(interp, $2, expr NEXT());
+
+    if (PMC_IS_NULL(sub))
+        real_exception(interp, NULL, GLOBAL_NOT_FOUND,
+            "Could not invoke non-existent sub %Ss", $2);
+
+    $1 = sub;
+}
+
 =back
 
 =head1 COPYRIGHT

Reply via email to