Re: [PATCH] Don't mix definitions and expressions in SRFI-9

[Andreas Rottmann]

address@hidden (Ludovic Courtès) writes:

> Hi,
>
> Andy Wingo <address@hidden> writes:
>
>> On Sun 06 Mar 2011 23:26, address@hidden (Ludovic Courtès) writes:
>>
>>> Andreas Rottmann <address@hidden> writes:
>>>
>>>> The expansion of `define-inlinable' contained an expression, which made
>>>> SRFI-9's `define-record-type' fail in non-toplevel contexts ("definition
>>>> used in expression context").
>>>
>>> SRFI-9 says “Record-type definitions may only occur at top-level”, and
>>> I’m inclined to stick to it.  If we diverge, then people could write
>>> code thinking it’s portable SRFI-9 code while it’s not.
>>
>> Does anyone actually care about this?  We provide many compatible
>> extensions to standard interfaces.  It seems like this would be an
>> "unnecessary restriction which makes `let-record-type' seem necessary".
>
> OK, I lost.  ;-)
>
> But, can we:
>
>   1. Document the extension.
>
>   2. Choose PROC-NAME such that -Wunused-toplevel won’t complain.
>      There’s a trick for this: if it contains white space, then
>      -Wunused-toplevel won’t complain; however, it has to be generated
>      deterministically because it can appear in other compilation units,
>      so we can’t use ‘generate-temporaries’ here.
>
I think the attached version of the patch takes your suggestions into
account.

From: Andreas Rottmann <address@hidden>
Subject: Don't mix definitions and expressions in SRFI-9

The expansion of `define-inlinable' contained an expression, which made
SRFI-9's `define-record-type' fail in non-toplevel contexts ("definition
used in expression context").

* module/srfi/srfi-9.scm (define-inlinable): Get rid of apparently
  useless expression in the expansion, so the expansion yields only
  definitions.  At the same time, use a space in the generated names to
  lessen the chances of name conflicts, also avoiding -Wunused-toplevel
  warnings.
* test-suite/tests/srfi-9.test (non-toplevel): New test verifying that
  `define-record-type' works in non-toplevel context as well.
* doc/ref/srfi-modules.texi (SRFI-9 - define-record-type): Add
  subsubsection noting that Guile does not enforce top-levelness.

---
 doc/ref/srfi-modules.texi    |    9 ++++++++-
 module/srfi/srfi-9.scm       |    8 +++++---
 test-suite/tests/srfi-9.test |   12 +++++++++++-
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/doc/ref/srfi-modules.texi b/doc/ref/srfi-modules.texi
index bda7cbb..eab8779 100644
--- a/doc/ref/srfi-modules.texi
+++ b/doc/ref/srfi-modules.texi
@@ -1,6 +1,6 @@
 @c -*-texinfo-*-
 @c This is part of the GNU Guile Reference Manual.
address@hidden Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2006, 
2007, 2008, 2009, 2010
address@hidden Copyright (C)  1996, 1997, 2000, 2001, 2002, 2003, 2004, 2006, 
2007, 2008, 2009, 2010, 2011
 @c   Free Software Foundation, Inc.
 @c See the file guile.texi for copying conditions.
 
@@ -1927,6 +1927,13 @@ The functions created by @code{define-record-type} are 
ordinary
 top-level @code{define}s.  They can be redefined or @code{set!} as
 desired, exported from a module, etc.
 
address@hidden Non-toplevel Record Definitions
+
+The SRFI-9 specification explicitly disallows record definitions in a
+non-toplevel context, such as inside @code{lambda} body or inside a
address@hidden block.  However, Guile's implementation does not enforce that
+restriction.
+
 @unnumberedsubsubsec Custom Printers
 
 You may use @code{set-record-type-printer!} to customize the default printing
diff --git a/module/srfi/srfi-9.scm b/module/srfi/srfi-9.scm
index 80c3b60..fad570b 100644
--- a/module/srfi/srfi-9.scm
+++ b/module/srfi/srfi-9.scm
@@ -1,6 +1,6 @@
 ;;; srfi-9.scm --- define-record-type
 
-;;     Copyright (C) 2001, 2002, 2006, 2009, 2010 Free Software Foundation, 
Inc.
+;;     Copyright (C) 2001, 2002, 2006, 2009, 2010, 2011 Free Software 
Foundation, Inc.
 ;;
 ;; This library is free software; you can redistribute it and/or
 ;; modify it under the terms of the GNU Lesser General Public
@@ -69,9 +69,12 @@
   ;; the macro expansion, whereas references in non-call contexts refer to
   ;; the procedure.  Inspired by the `define-integrable' macro by Dybvig et al.
   (lambda (x)
+    ;; Use a space in the prefix to avoid potential -Wunused-toplevel
+    ;; warning
+    (define prefix (string->symbol "% "))
     (define (make-procedure-name name)
       (datum->syntax name
-                     (symbol-append '% (syntax->datum name)
+                     (symbol-append prefix (syntax->datum name)
                                     '-procedure)))
 
     (syntax-case x ()
@@ -81,7 +84,6 @@
          #`(begin
              (define (proc-name formals ...)
                body ...)
-             proc-name ;; unused
              (define-syntax name
                (lambda (x)
                  (syntax-case x ()
diff --git a/test-suite/tests/srfi-9.test b/test-suite/tests/srfi-9.test
index cf933a8..f8006c4 100644
--- a/test-suite/tests/srfi-9.test
+++ b/test-suite/tests/srfi-9.test
@@ -1,7 +1,7 @@
 ;;;; srfi-9.test --- Test suite for Guile's SRFI-9 functions. -*- scheme -*-
 ;;;; Martin Grabmueller, 2001-05-10
 ;;;;
-;;;; Copyright (C) 2001, 2006, 2007, 2010 Free Software Foundation, Inc.
+;;;; Copyright (C) 2001, 2006, 2007, 2010, 2011 Free Software Foundation, Inc.
 ;;;; 
 ;;;; This library is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -93,3 +93,13 @@
   ;; prior to guile 1.6.9 and 1.8.1 this wan't enforced
   (pass-if-exception "set-y! on bar" exception:wrong-type-arg
      (set-y! b 99)))
+
+(with-test-prefix "non-toplevel"
+    
+  (define-record-type :frotz (make-frotz a b) frotz?
+    (a frotz-a) (b frotz-b set-frotz-b!))
+
+  (pass-if "construction"
+    (let ((frotz (make-frotz 1 2)))
+      (and (= (frotz-a frotz) 1)
+           (= (frotz-b frotz) 2)))))
-- 
tg: (d59dd06..) t/srfi-9-fix (depends on: stable-2.0)

Thanks, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>