diff mbox series

[bug#63135,v2,1/5] records: match-record: Raise a syntax error if TYPE is nonexistent.

Message ID 20230428191905.13860-2-paren@disroot.org
State New
Headers show
Series MATCH-RECORD improvements | expand

Commit Message

\( April 28, 2023, 7:19 p.m. UTC 
* guix/records.scm (match-record): Raise a human-compherensible syntax error
  if the given record type identifier is unbound.

Co-authored-by: Josselin Poiret <dev@jpoiret.xyz>
---
 guix/records.scm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Ludovic Courtès May 19, 2023, 3:22 p.m. UTC | #1 
Hi,

Thanks for these much welcome improvements!

"(" <paren@disroot.org> skribis:

> * guix/records.scm (match-record): Raise a human-compherensible syntax error
>   if the given record type identifier is unbound.

[...]

>  (define-syntax map-fields
>    (lambda (x)
> -    (syntax-violation 'map-fields "bad use of syntactic keyword" x x)))
> +    (syntax-case x ()
> +      ((_ type within)
> +       (syntax-violation (syntax->datum #'within)
> +                         "undefined guix record-type"
> +                         #'type))

How about “invalid record type identifier”?

(Rule of thumb: never use “Guix” in messages and interfaces.)

> -                     ((_ map-fields macro)
> +                     ((_ (map-fields _ _) macro)
>                        #'(macro (field ...)))
>                       (id
>                        (identifier? #'id)
> @@ -595,7 +600,7 @@ (define-syntax match-record-inner
>         #'(let-syntax ((field-offset (syntax-rules ()
>  			              ((_ f)
>                                         (lookup-field field 0 f)))))
> -           (let* ((offset (type map-fields field-offset))
> +           (let* ((offset (type (map-fields type match-record) field-offset))

There’s always a tradeoff; not a strong opinion but I’d lean towards
keeping the macro unchanged (thus a bit simpler) and simply changing the
default ‘syntax-violation’ message above.

WDYT?

Ludo’.
diff mbox series

Patch

diff --git a/guix/records.scm b/guix/records.scm
index 7d43b064d8..d8966998c1 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -105,7 +105,12 @@  (define (report-duplicate-field-specifier name ctor)
 
 (define-syntax map-fields
   (lambda (x)
-    (syntax-violation 'map-fields "bad use of syntactic keyword" x x)))
+    (syntax-case x ()
+      ((_ type within)
+       (syntax-violation (syntax->datum #'within)
+                         "undefined guix record-type"
+                         #'type))
+      (_ (syntax-violation 'map-fields "bad use of syntactic keyword" x x)))))
 
 (define-syntax-parameter this-record
   (lambda (s)
@@ -459,7 +464,7 @@  (define-syntax type
                    "This macro lets us query record type info at
 macro-expansion time."
                    (syntax-case s (map-fields)
-                     ((_ map-fields macro)
+                     ((_ (map-fields _ _) macro)
                       #'(macro (field ...)))
                      (id
                       (identifier? #'id)
@@ -595,7 +600,7 @@  (define-syntax match-record-inner
        #'(let-syntax ((field-offset (syntax-rules ()
 			              ((_ f)
                                        (lookup-field field 0 f)))))
-           (let* ((offset (type map-fields field-offset))
+           (let* ((offset (type (map-fields type match-record) field-offset))
                   (variable (struct-ref record offset)))
              (match-record-inner record type (rest ...) body ...))))
       ((_ record type (field rest ...) body ...)