srfi-9 record type checking

srfi-9 record type checking

[Kevin Ryde]

Speaking of srfi-9 records (Ludovic on guile-user), I notice the spec
calls for an accessor used on a wrong type record to be an error but
currently guile returns #f for that.  Eg.

    (use-modules (srfi srfi-9))

    (define-record-type foo
      (make-foo f1 f2)
      foo?
      (f1 get-f1 set-f1)
      (f2 get-f2 set-f2))

    (define-record-type bar
      (make-bar b1 b2)
      bar?
      (b1 get-b1 set-b1)
      (b2 get-b2 set-b2))

    (define f (make-foo 1 2))

    (pk (get-b1 f))

I guess `define-record-type' should be making a stricter procedure
than `record-accessor' gives.

I got bitten by this using `record-accessor' the other day.  For some
reason I thought it had strict type checking and scratched my head for
a while until realizing I was passing a wrong record and getting back
#f.  Is that #f a feature?  Would there be scope to add an option to
ask for a strict checking version?  Or make that the default?


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> I guess `define-record-type' should be making a stricter procedure
> than `record-accessor' gives.
>
> I got bitten by this using `record-accessor' the other day.  For some
> reason I thought it had strict type checking and scratched my head for
> a while until realizing I was passing a wrong record and getting back
> #f.  Is that #f a feature?  Would there be scope to add an option to
> ask for a strict checking version?  Or make that the default?

I don't think we can add an optional argument to `record-accessor' while
preserving the current behavior by default.  That optional argument
would have to be more than just a default value: typically it would have
to be a wrong-type-arg handling procedure, and this would add too much
overhead IMO (OTOH, one may argue that we shouldn't worry too much about
overhead in error conditions).

Instead, we could perhaps add `record-type-strict-accessor'?

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Kevin Ryde <user42@zip.com.au> writes:
>
>> I guess `define-record-type' should be making a stricter procedure
>> than `record-accessor' gives.
>>
>> I got bitten by this using `record-accessor' the other day.  For some
>> reason I thought it had strict type checking and scratched my head for
>> a while until realizing I was passing a wrong record and getting back
>> #f.  Is that #f a feature?  Would there be scope to add an option to
>> ask for a strict checking version?  Or make that the default?
>
> I don't think we can add an optional argument to `record-accessor' while
> preserving the current behavior by default.  That optional argument
> would have to be more than just a default value: typically it would have
> to be a wrong-type-arg handling procedure, and this would add too much
> overhead IMO (OTOH, one may argue that we shouldn't worry too much about
> overhead in error conditions).
>
> Instead, we could perhaps add `record-type-strict-accessor'?

I vote for just fixing record-accessor to be strict by default;
i.e. treating current behaviour as a bug.  I'd be surprised if anyone
was relying on this, and if they are it's easy to code a workaround.

Regards,
      Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Kevin Ryde]

Neil Jerram <neil@ossau.uklinux.net> writes:
>
> I vote for just fixing record-accessor to be strict by default;

That's what seems best to me.

record-modifier may be more obvious, I think it silently does nothing
if given a wrong type record, which is unlikely to be what anyone
wants!


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> I vote for just fixing record-accessor to be strict by default;
> i.e. treating current behaviour as a bug.  I'd be surprised if anyone
> was relying on this, and if they are it's easy to code a workaround.

The issue is that this would impact people that use records directly
(i.e., not via SRFI-9) and although this `#f' behavior is undocumented,
they may well have come to rely on it.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> I vote for just fixing record-accessor to be strict by default;
>> i.e. treating current behaviour as a bug.  I'd be surprised if anyone
>> was relying on this, and if they are it's easy to code a workaround.
>
> The issue is that this would impact people that use records directly
> (i.e., not via SRFI-9) and although this `#f' behavior is undocumented,
> they may well have come to rely on it.

Yes, that is indeed the issue.  I just think that (i) your "may" is
extremely unlikely, and/because (ii) the current behaviour is
sufficiently odd that we can see it as a bug - for which there is
obviously no back-compatibility requirement - instead of as
"behaviour", and (iii) the nature of 'record-accessor' is such that it
can be fixed up in just one place if someone really does want the old
behaviour:

(define new-record-accessor record-accessor)

(set! record-accessor
  (lambda (type field)
    (lambda (obj)
      (and ((record-predicate type) obj)
           ((new-record-accessor type field) obj)))))

So on balance I don't think we need to provide complete compatibility
within Guile here.

Regards,
     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> (define new-record-accessor record-accessor)
>
> (set! record-accessor
>   (lambda (type field)
>     (lambda (obj)
>       (and ((record-predicate type) obj)
>            ((new-record-accessor type field) obj)))))
>
> So on balance I don't think we need to provide complete compatibility
> within Guile here.

Right, I think you convinced me.  :-)

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Kevin Ryde]

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

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> Neil Jerram <neil@ossau.uklinux.net> writes:
>>
>> So on balance I don't think we need to provide complete compatibility
>> within Guile here.
>
> Right, I think you convinced me.  :-)

In absense of violent objections I made the change below.  I didn't do
the same in the 1.6 branch, there I only amended the srfi-9
implementation.

Is the local-eval stuff supposed to minimize the amount of environment
or whatever captured by the returned functions?  Guessing that's so I
put the type check in a helper function.



[-- Attachment #2: boot-9.scm.records.diff --]
[-- Type: text/plain, Size: 1200 bytes --]

--- boot-9.scm.~1.356.2.1.~	2006-05-09 10:34:24.000000000 +1000
+++ boot-9.scm	2006-08-02 08:40:13.000000000 +1000
@@ -429,13 +429,20 @@
 (define (record-predicate rtd)
   (lambda (obj) (and (struct? obj) (eq? rtd (struct-vtable obj)))))
 
+(define (%record-type-check rtd obj)  ;; private helper
+  (or (eq? rtd (record-type-descriptor obj))
+      (scm-error 'wrong-type-arg "%record-type-check"
+		 "Wrong type record (want `~S'): ~S"
+		 (list (record-type-name rtd) obj)
+		 #f)))
+
 (define (record-accessor rtd field-name)
   (let* ((pos (list-index (record-type-fields rtd) field-name)))
     (if (not pos)
 	(error 'no-such-field field-name))
     (local-eval `(lambda (obj)
-		   (and (eq? ',rtd (record-type-descriptor obj))
-			(struct-ref obj ,pos)))
+		   (%record-type-check ',rtd obj)
+		   (struct-ref obj ,pos))
 		the-root-environment)))
 
 (define (record-modifier rtd field-name)
@@ -443,8 +450,8 @@
     (if (not pos)
 	(error 'no-such-field field-name))
     (local-eval `(lambda (obj val)
-		   (and (eq? ',rtd (record-type-descriptor obj))
-			(struct-set! obj ,pos val)))
+		   (%record-type-check ',rtd obj)
+		   (struct-set! obj ,pos val))
 		the-root-environment)))
 
 

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Ludovic Courtès]

Hi,

Kevin Ryde <user42@zip.com.au> writes:

> In absense of violent objections I made the change below.  I didn't do
> the same in the 1.6 branch, there I only amended the srfi-9
> implementation.

Cool, thanks.  Maybe this should be described in the `NEWS' file?

> Is the local-eval stuff supposed to minimize the amount of environment
> or whatever captured by the returned functions?  Guessing that's so I
> put the type check in a helper function.

Perhaps, but `the-root-environment' is pretty much the same as the
environment in which `record-accessor' is defined, right?

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> Maybe this should be described in the `NEWS' file?

Yep, done.  I left the manual alone it already calls for a "record of
the appropriate type" as the argument to the created funcs.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: srfi-9 record type checking

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> I don't think we can add an optional argument to `record-accessor' while
> preserving the current behavior by default.  That optional argument
> would have to be more than just a default value: typically it would have
> to be a wrong-type-arg handling procedure,

I had in mind a flag, or maybe a keyword,

	(record-accessor my-type 'strict)

just to ask for the different flavour procedure coming back.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel