bug#15691: ,br doesn't work for generic functions

bug#15691: ,br doesn't work for generic functions

[Jordy Dickinson]

# What I did:

I used ,br with a generic function, specifically I entered ",br initialize".

# What I expected:

I wasn't sure what to expect, given that generic functions can have
many implementations, but since there was no error and a "generic"
object appeared in the output of ",traps", I expected at least that
when _some_ initialize method was called I would be put into a
debugging prompt.

# What actually happened:

When I called a function that called the initialize method ("make" in
this particular case), it completed successfully, without any debug
prompt.

bug#15691: ,br doesn't work for generic functions

[Ian Price]

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


Jordy Dickinson <jordy.dickinson@gmail.com> writes:

> # What I expected:
>
> I wasn't sure what to expect, given that generic functions can have
> many implementations, but since there was no error and a "generic"
> object appeared in the output of ",traps", I expected at least that
> when _some_ initialize method was called I would be put into a
> debugging prompt.

Indeed, there are two reasonable behaviours here.

1. We stop before method specialisation in the "top level" of the
function returned by (procedure <some-generic>). This is the behaviour I
would expect for an arbitrary applicable struct.

2. We stop after method specialisation. This requires goops specific
hackery, but is probably more helpful if we could pull it off.

>
> # What actually happened:
>
> When I called a function that called the initialize method ("make" in
> this particular case), it completed successfully, without any debug
> prompt.

The reason this happens is that in the procedure `frame-matcher' in
(system vm traps), it tests for the equivalence of the procedure passed
in, with the procedure in the frame with eq?. In the first instance, the
procedure is an applicable struct, the latter however is the function
returned by (procedure that-applicable-struct).

We could either fix this a bit further up the stack, by making sure that
we extract the procedure from applicable structs, we could extract it
in `frame-matcher'.

I think `frame-matcher' is the right place to fix this, rather than
further up the stack. Various procedures in (system vm traps) are
documented as taking a procedure, which IMO should include applicable
structs.

I've attached a patch which handles this, implementing behaviour (1)
above.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"


[-- Attachment #2: ,br fix --]
[-- Type: #("text/x-patch" 2 3 (face ido-hacks-flex-match) 7 8 (face ido-hacks-flex-match) 8 9 (face ido-hacks-flex-match) 9 10 (face ido-hacks-flex-match) 10 11 (face ido-hacks-flex-match)), Size: 1703 bytes --]

From 3a117b22b10c9577868fd22f88d1cad2e2ec32a0 Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Thu, 24 Oct 2013 05:51:47 +0100
Subject: [PATCH] Fix trap handlers to handle applicable structs.

Reported by Jordy Dickinson <jordy.dickinson@gmail.com>.
Fixes <http://bugs.gnu.org/15691>.

* module/system/vm/traps.scm (frame-matcher): Extract procedure when
  proc is an applicable struct.
---
 module/system/vm/traps.scm | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/module/system/vm/traps.scm b/module/system/vm/traps.scm
index cccd6ea..b65e034 100644
--- a/module/system/vm/traps.scm
+++ b/module/system/vm/traps.scm
@@ -109,15 +109,18 @@
   ((new-disabled-trap vm enable disable) frame))
 
 (define (frame-matcher proc match-objcode?)
-  (if match-objcode?
-      (lambda (frame)
-        (let ((frame-proc (frame-procedure frame)))
-          (or (eq? frame-proc proc)
-              (and (program? frame-proc)
-                   (eq? (program-objcode frame-proc)
-                        (program-objcode proc))))))
-      (lambda (frame)
-        (eq? (frame-procedure frame) proc))))
+  (let ((proc (if (struct? proc)
+                  (procedure proc)
+                  proc)))
+    (if match-objcode?
+        (lambda (frame)
+          (let ((frame-proc (frame-procedure frame)))
+            (or (eq? frame-proc proc)
+                (and (program? frame-proc)
+                     (eq? (program-objcode frame-proc)
+                          (program-objcode proc))))))
+        (lambda (frame)
+          (eq? (frame-procedure frame) proc)))))
 
 ;; A basic trap, fires when a procedure is called.
 ;;
-- 
1.7.11.7

bug#15691: ,br doesn't work for generic functions

[Ian Price]

Ian Price <ianprice90@googlemail.com> writes:

> We could either fix this a bit further up the stack, by making sure that
> we extract the procedure from applicable structs, we could extract it
> in `frame-matcher'.

It did not occur to me before I sent it, but there is one other
reasonable behaviour, which would be to store the applicable struct as
the frame procedure.  This might actually be preferable to the fix I
posted, but it will hacking in C.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

bug#15691: ,br doesn't work for generic functions

[Ian Price]

I pushed the patch from the 24 of October (commit
3981feaa11aa0d866ff1d99128f0ace3). It's not perfect,
but I'm marking this one done. Sorry it took a while.


-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"