[PATCH] records: Check for missing mandatory innate fields when inheriting.

  • Open
  • quality assurance status badge
Details
One participant
  • Skyler Ferris
Owner
unassigned
Submitted by
Skyler Ferris
Severity
normal
S
S
Skyler Ferris wrote on 11 Dec 2023 23:09
(address . guix-patches@gnu.org)(name . Skyler Ferris)(address . skyvine@protonmail.com)
5a0f596c5bbcc75e3ca50d37066f42baca354236.1702330063.git.skyvine@protonmail.com
This gives the user a more helpful error message if they inherit from
another instance but forget to supply a value for a field which is
innate and has no default.

* guix/records.scm (record-inheritance): Check for missing innate fields

Change-Id: I08b898d8da551af50aa5b0b60187dcad8d259bc1
---
This fixes issue #67785 by providing the same error message that is
provided when the value is not inherited.

After applying this patch `make check` had one failure, which seems to
be unrelated. It was the test "pypi->guix-package, no wheel"; for some
reason, the download of the wheel file failed. All other tests, in
particular the tests in `tests/records.scm`, passed.

I additionally manually inspected the values computed in the `let*`
against a type that contained 4 fields, for each combination of
mandatory/non-mandatory and optional/non-optional, and they contained
the expected values.

guix/records.scm | 9 +++++++++
1 file changed, 9 insertions(+)

Toggle diff (24 lines)
diff --git a/guix/records.scm b/guix/records.scm
index f4d12a861d..109c5cf4dc 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -161,6 +161,15 @@ (define-syntax make-syntactic-constructor
(record-error 'name s "extraneous field initializers ~a"
unexpected)))
+ ;; Make sure all mandatory and innate fields are included.
+ (let* ((fields (map (compose car syntax->datum) field+value))
+ (optional (map (compose car syntax->datum) 'defaults))
+ (mandatory (lset-difference eq? '(expected ...) optional))
+ (mandatory+innate (lset-intersection eq? mandatory 'innate))
+ (missing (lset-difference eq? mandatory+innate fields)))
+ (when (pair? missing)
+ (record-error 'name s "missing field initializers ~a" missing)))
+
#`(make-struct/no-tail type
#,@(map (lambda (field index)
(or (field-inherited-value field)

base-commit: 2b782f67266b42bb40015bd23ce2443be2f9b01f
--
2.41.0
S
S
Skyler Ferris wrote on 28 Dec 2023 01:30
Re: records: Check for missing mandatory innate fields when, inheriting.
(address . 67788@debbugs.gnu.org)
aabc0db6-418f-4d2f-8528-00c44ec5d234@protonmail.com
his gives the user a more helpful error message if they inherit from
another instance but forget to supply a value for a field which is
innate and has no default.

* guix/records.scm (record-inheritance): Check for missing innate fields

Change-Id: I08b898d8da551af50aa5b0b60187dcad8d259bc1
---
# This version of the commit addse a test case which confirms that
# theuseful commit message is given. It is based on the test immediately
# above it, which checks for the error message in the non-innate case.
#
# Additionally, I can now confirm that all tests pass with this commit.
# The failures I noted previously were indeed environmental problems and
# have been resolved locally (and were unrelated to the commit).
 guix/records.scm  |  9 +++++++++
 tests/records.scm | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+)

Toggle diff (59 lines)
diff --git a/guix/records.scm b/guix/records.scm
index f4d12a861d..109c5cf4dc 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -161,6 +161,15 @@ (define-syntax make-syntactic-constructor
                (record-error 'name s "extraneous field initializers ~a"
                              unexpected)))

+           ;; Make sure all mandatory and innate fields are included.
+           (let* ((fields           (map (compose car syntax->datum)
field+value))
+                  (optional         (map (compose car syntax->datum)
'defaults))
+                  (mandatory        (lset-difference eq? '(expected
...) optional))
+                  (mandatory+innate (lset-intersection eq? mandatory
'innate))
+                  (missing          (lset-difference eq?
mandatory+innate fields)))
+             (when (pair? missing)
+               (record-error 'name s "missing field initializers ~a"
missing)))
+
            #`(make-struct/no-tail type
                           #,@(map (lambda (field index)
                                     (or (field-inherited-value field)
diff --git a/tests/records.scm b/tests/records.scm
index 5464892d3b..39f4bfdf9c 100644
--- a/tests/records.scm
+++ b/tests/records.scm
@@ -402,6 +402,24 @@ (define (location-alist loc)
            (string-match "missing .*initialize.*baz" message)
            (equal? form '(foo))))))

+(test-assert "define-record-type* & innate & missing initializers"
+  (catch 'syntax-error
+    (lambda ()
+      (eval '(begin
+               (define-record-type* <foo> foo make-foo
+                 foo?
+                 (bar foo-bar (default 42))
+                 (baz foo-baz (innate)))
+
+               (let ((base (foo (bar #t) (baz #t))))
+                 (foo (inherit base))))
+            (test-module))
+      #f)
+    (lambda (key proc message location form . args)
+      (and (eq? proc 'foo)
+           (string-match "missing .*initialize.*baz" message)
+           (equal? form '(foo (inherit base)))))))
+
 (test-assert "define-record-type* & extra initializers"
   (catch 'syntax-error
     (lambda ()

base-commit: fe86819d8bde674766659c22b215d3a689a8026e
--
2.41.0
S
S
Skyler Ferris wrote on 28 Dec 2023 03:05
(address . 67788@debbugs.gnu.org)
d8ece7cc-1b3a-440b-bc4a-f6c9e4a45d6b@protonmail.com
It looks like this update has the some non-breaking space problem that
also occurred on the other patch I updated, 87786. I'll send a fixed
version once I am able.
S
S
skyvine wrote on 13 Jan 20:00 +0100
Re: doc: Add documentation for define-record-type*
(address . 67788@debbugs.gnu.org)(name . Skyler Ferris)(address . skyvine@protonmail.com)
43d0fdc9635394538ffee724beebe885d357bc59.1703713111.git.skyvine@protonmail.com
From: Skyler Ferris <skyvine@protonmail.com>

I wrote this documentation by using the docstring in guix/record.scm as a
starting point and expanding on it/reformatting on it to be appropriate
for inclusion in a manual.

* doc/guix.texi: Add sections describing the typical
usage and API for define-record-type*

Change-Id: I19e7220553d10652c794e6e0172b2c9ee961f54f
---
# This version of the patch updates the commit message to explain that I
# wrote the documentation by using the docstring in guix/records.scm as
# a starting point. This process seemed appropriate to me because it is
# being incorporated into the same project for the same purpose. Most of
# the commit is new content, but some of it is shared with the docstring
# - for example the "thing" struct definition and the explanation of
# struct fields. I think that it is useful for the manual and the
# docstrings to share content when appropriate to avoid accidentally
# communicating contradictory ideas to different audiences, but the
# relationship should still be acknowledged. If there is a more formal
# manner in which I should indicate this please let me know and I will
# be happy to update the commit. This version does not change anything
# other than the commit message.
doc/contributing.texi | 1 -
doc/guix.texi | 274 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 274 insertions(+), 1 deletion(-)

Toggle diff (299 lines)
diff --git a/doc/contributing.texi b/doc/contributing.texi
index 7337f4bd58..d920b2589a 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -1313,7 +1313,6 @@ Data Types and Pattern Matching
notably the fact that it is hard to read, error-prone, and a hindrance
to proper type error reports.
-@findex define-record-type*
@findex match-record
@cindex pattern matching
Guix code should define appropriate data types (for instance, using
diff --git a/doc/guix.texi b/doc/guix.texi
index e61a893af9..3cb15eeca3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12606,6 +12606,280 @@ G-Expressions
@code{(*approximate*)}, but this may change.
@end deffn
+@node Records in Guix
+@section Records in Guix
+Guix uses @code{define-record-type*} to define structures with a lispy format.
+Packages, operating systems, etc are all defined with
+@code{define-record-type*} facilities. If one was using this facility to
+define preferences for a text editor, it might look like this:
+
+@lisp
+;; The only valid emulation modes are the symbol 'emacs, the symbol 'vim, or
+;; the boolean #f. As a convenience to the user, if they pass in a string
+;; first convert it to a symbol and accept it if it is valid.
+(define (sanitize-emulation-mode value)
+ (let ((symbolized-value (cond ((not value) #f)
+ ((string? value) (string->symbol value))
+ (#t value))))
+ (unless (or (not symbolized-value)
+ (eq? symbolized-value 'emacs)
+ (eq? symbolized-value 'vim))
+ (throw 'bad-emulation-made
+ (format #f "Unrecognized emulation mode: ~s" value)))
+ symbolized-value))
+
+(define-record-type*
+ <editor-preferences> editor-preferences make-editor-preferences
+ editor-preferences? this-editor-preferences
+ (background-color editor-preferences-background-color
+ (default "000000"))
+ (text-color editor-preferences-text-color
+ (default "FFFFFF"))
+ (emulation-mode editor-preferences-emulation-mode
+ (default #f)
+ (sanitize sanitize-emulation-mode)))
+@end lisp
+
+A user could then define their preferences like this:
+
+@lisp
+(define my-preferences
+ (editor-preferences
+ (background-color "222222")
+ (emulation-mode 'vim)))
+@end lisp
+
+The value contained in @code{my-preferences} contains a custom
+@code{background-color} and @code{emulation-mode}, but keeps the default
+@code{text-color} (@code{"FFFFFF"}). If an invalid @code{emulation-mode} had
+been specified, for example if the user passed in @code{"vi"} instead of
+@code{"vim"}, @code{sanitize-emulation-mode} would immediately throw an error.
+
+The program can access values like this:
+
+@lisp
+(editor-preferences-background-color my-preferences)
+@result{} "222222"
+(editor-preferences-text-color my-preferences)
+@result{} "FFFFFF"
+(editor-preferences-emulation-mode my-preferences)
+@result{} 'vim
+@end lisp
+
+There is no way to define setters (all instances are immutable).
+
+@node Record Inheritance
+@subsection Record Inheritance
+It is also possible to inherit from previously defined instances when creating
+new ones. Continuing with the editor example, someone might want to base their
+preferences on their friend's preferences but customize a value:
+
+@lisp
+(define friends-preferences
+ (editor-preferences
+ (inherit my-preferences)
+ (emulation-mode 'emacs)))
+@end lisp
+
+This keeps the same @code{background-color} and @code{text-color} that are
+contained in @code{my-preferences} but changes the @code{emulation-mode} to
+be @code{'emacs} instead of @code{'vim}.
+
+Sometimes it does not make sense for a field to be inherited. Suppose that the
+@code{<editor-preferences>} type is updated to contain a username so that a
+friendly greeting can be displayed when the program starts up:
+
+@lisp
+;; Usernames must be strings. It would be strange to pass a username as a
+;; symbol, so throw an error in case the user meant to pass in a variable's
+;; value instead of a literal symbol.
+(define (sanitize-username value)
+ (unless (string? value)
+ (throw 'bad-username
+ (format #f "Usernames must be strings! Got: ~s" value)))
+ value)
+
+(define (sanitize-emulation-mode value)
+ (let ((symbolized-value (cond ((not value) #f)
+ ((string? value) (string->symbol value))
+ (#t value))))
+ (unless (or (not symbolized-value)
+ (eq? symbolized-value 'emacs)
+ (eq? symbolized-value 'vim))
+ (throw 'bad-emulation-made
+ (format #f "Unrecognized emulation mode: ~s" value)))
+ symbolized-value))
+
+(define-record-type*
+ <editor-preferences> editor-preferences make-editor-preferences
+ editor-preferences? this-editor-preferences
+ (username editor-preferences-username
+ (innate)
+ (sanitize sanitize-username))
+ (background-color editor-preferences-background-color
+ (default "000000"))
+ (text-color editor-preferences-text-color
+ (default "FFFFFF"))
+ (emulation-mode editor-preferences-emulation-mode
+ (default #f)
+ (sanitize sanitize-emulation-mode)))
+@end lisp
+
+There are a couple of differences in the new @code{username} field compared to
+the fields we looked at earlier. It is marked as @code{innate}, which means
+that it will not be inherited. For example, consider what would happen if we
+tried to define new instances like this:
+
+@lisp
+(define my-preferences
+ (editor-preferences
+ (username "my-username")
+ (background-color "222222")
+ (emulation-mode 'vim)))
+
+(define friends-preferences
+ (editor-preferences
+ (inherit my-preferences)
+ (emulation-mode 'emacs)))
+@end lisp
+
+While the @code{friends-preferences} instance still inherits the values for
+@code{background-color} and @code{text-color}, it will not inherit the value
+for @code{username}. Furthermore, as the @code{username} field does not define
+a default value the attempted creation of @code{friends-preferences} will
+actually throw an error. Instead, we could do this:
+
+@lisp
+(define my-preferences
+ (editor-preferences
+ (username "my-username")
+ (background-color "222222")
+ (emulation-mode 'vim)))
+
+(define friends-preferences
+ (editor-preferences
+ (inherit my-preferences)
+ (username "friends-username")
+ (emulation-mode 'emacs)))
+@end lisp
+
+@node @code{define-record-type*} Reference
+@subsection @code{define-record-type*} Reference
+@defmac define-record-type* name syntactic-constructor constructor predicate this-identifier fields ...
+
+Define a new record type and associated helpers.
+
+@table @var
+@item name
+A symbol used to name the type, as would normally be provided to a plain
+@code{define-record-type} form. For example, @code{<package>}.
+
+@item syntactic-constructor
+A symbol that will be used to define the user-facing constructor. For example,
+the symbol @code{package} is the syntactic constructor for the @code{<package>}
+structure.
+
+@item constructor
+A symbol that will be used to define the traditional constructor. It is used in
+the implementation of the syntactic constructor, but will not typically be used
+elsewhere. The traditional @code{make-name} (for example, @code{make-package})
+is a fine value to use here.
+
+@item predicate
+A symbol that will be used to test if a value is an instance of this record.
+For example, @code{package?}.
+
+@item this-identifier
+This symbol can be used when defining fields that need to refer to the struct
+that contains them. For an example of this, see the @code{thunked} field
+property, below.
+
+@item fields
+A set of field specifiers which take the following form:
+
+@lisp
+(field-name field-getter properties ...)
+@end lisp
+
+Each of the properties must have one of the following forms:
+
+@table @code
+@item (default @var{value})
+Defines the default value for the field, if the user does not specify one using
+the syntactic constructor.
+
+@item (innate)
+Fields marked as innate will not be inherited from parent objects (see
+Instantiating Records, below, for details of object inheritance).
+
+@item (sanitize @var{proc})
+The value given by the user will be passed into @var{proc} before being stored
+in the object. For example, consider this struct definition:
+
+@lisp
+(define-record-type* <thing> thing make-thing
+ thing?
+ this-thing
+ (name thing-name
+ (sanitize (lambda (value)
+ (cond ((string? value) value)
+ ((symbol? value) (symbol->string value))
+ (else (throw 'bad! value)))))))
+@end lisp
+
+When creating @code{thing} instances either a string or a symbol can be
+supplied but it will always be stored as a string:
+
+@lisp
+(string? (thing-name (thing (name "some-name"))))
+@result{} #t
+(string? (thing-name (thing (name 'some-name))))
+@result{} #t
+(thing (name 1994))
+@result{} Throw to key `bad!' with args `(1994)'.
+@end lisp
+
+@item (thunked)
+Fields marked as @code{thunked} will actually compute the field's value in the
+current dynamic extent which is useful when referring to fluids in a field's
+value. Furthermore, that thunk can access the record it belongs to via the
+@code{this-identifier}. For example:
+
+@lisp
+(define-record-type* <rectangle> rectangle make-rectangle
+ rectangle?
+ this-rectangle
+ (width rectangle-width)
+ (height rectangle-height)
+ (area rectangle-area (thunked)
+ (default (* (rectangle-width this-rectangle)
+ (rectangle-height this-rectangle)))))
+
+(define base-rectangle
+ (rectangle
+ (width 2)
+ (height 4)))
+
+(define derived-rectangle
+ (rectangle
+ (inherit base)
+ (width 6)))
+
+(rectangle-area base-rectangle)
+@result{} 8
+
+(rectangle-area derived-rectangle
+@result{} 24
+@end lisp
+
+@item (delayed)
+Fields marked as @code{delayed} are similar to @code{thunked} fields, except
+that they are effectively wrapped in a @code{(delay @dots{})} form. Note that
+delayed fields cannot use @code{this-identifier}.
+@end table
+@end table
+@end defmac
+
@node Invoking guix repl
@section Invoking @command{guix repl}
--
2.41.0
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 67788@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 67788
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch