Re: Quite sneaky bug in SRFI-9

[Ludovic Courtès]

Hi Andreas,

Andreas Rottmann <address@hidden> writes:

> Hi!   After being puzzled for some time, I found the root cause for an
> issue with my code was actually a quite serious bug in SRFI-9's
> define-inlinable' (here we go again -- but this time it's an actual
> bug, I promise ;-)):  The current implementation of `define-inlinable'
> duplicates actual parameters passed to the macros it generates, for
> example (note the `(find-next-solution! s 10000)' expression):
> scheme@(guile-user)> (tree-il->scheme (macroexpand '(solution?
> (find-next-solution! s 10000))))
> $6 = (if ((@@ (srfi srfi-9) struct?) (find-next-solution! s 10000))
>         ((@@ (srfi srfi-9) eq?)            ((@@ (srfi srfi-9)
> struct-vtable) (find-next-solution! s 10000))
>              (@@ (dorodango solver) solution))
>          #f)
>

Ouch, good catch!

> - It's certainly not as efficient as the current, broken implementation.
>  Ideally, the expansion would be a `let' block instead of the
>  `call-with-values' invocation.

Yes, that would be nice.  This patch seems to do the trick:

diff --git a/module/srfi/srfi-9.scm b/module/srfi/srfi-9.scm
index fad570b..54bc8a8 100644
--- a/module/srfi/srfi-9.scm
+++ b/module/srfi/srfi-9.scm
@@ -64,6 +64,12 @@
 
 (cond-expand-provide (current-module) '(srfi-9))
 
+(define-syntax letify
+  (syntax-rules ()
+    ((_ (var ...) (val ...) body ...)
+     (let ((var val) ...)
+       body ...))))
+
 (define-syntax define-inlinable
   ;; Define a macro and a procedure such that direct calls are inlined, via
   ;; the macro expansion, whereas references in non-call contexts refer to
@@ -80,15 +86,17 @@
     (syntax-case x ()
       ((_ (name formals ...) body ...)
        (identifier? #'name)
-       (with-syntax ((proc-name (make-procedure-name #'name)))
+       (with-syntax ((proc-name  (make-procedure-name #'name))
+                     ((args ...) (generate-temporaries #'(formals ...))))
          #`(begin
              (define (proc-name formals ...)
                body ...)
              (define-syntax name
                (lambda (x)
                  (syntax-case x ()
-                   ((_ formals ...)
-                    #'(begin body ...))
+                   ((_ args ...)
+                    #'(letify (formals ...) (args ...)
+                              (begin body ...)))
                    (_
                     (identifier? x)
                     #'proc-name))))))))))

I can’t think of any way to do it more elegantly.

> Has anyone done benchmarks of the benefits of the current (broken)
> SRFI-9 accessors and predicates vs. regular procedures on real code?
> Given the argument duplication issue, it might be the case that
> `define-inlinable' actually slowed things down :-).

I benchmarked it indirectly via ‘vlist-cons’ et al. in ‘vlists.bm’.

The bonus is that there are special opcodes for ‘struct-vtable’,
‘make-struct’, and ‘struct-ref’.  So when you inline constructors and
accessors, you also get to use directly those opcodes, which makes quite
a difference.

I would like to keep this property.  Not doing this gives an incentive
to using lists everywhere since ‘car’, ‘pair?’, etc. are open-coded.

Thanks!

Ludo’.