[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
?