Record special field abstraction leakage

  • Open
  • quality assurance status badge
Details
2 participants
  • Leo Prikler
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important

Debbugs page

L
L
Ludovic Courtès wrote on 26 Mar 2019 02:38
(address . bug-Guix@gnu.org)
87zhpiht6k.fsf@gnu.org
The changes I made in version-control.scm and gnucash.scm in commit
e6301fb76d0a8d931ece2e18d197e3c2cc53fc6c revealed an abstraction leakage
I wasn’t aware of: there’s a pattern where users “see” that thunked
fields are thunked:

(package
;; …
(inputs …)
(arguments `(foo bar ,(inputs) …))) ;<- here ‘inputs’ is seen as a thunk

Fortunately I could only find two occurrences of this and this use case
is more elegantly replaced by:

(package-inputs this-record)

… which also has better semantics. It’s remains a bug, though.

Ludo’.
L
L
Ludovic Courtès wrote on 4 Apr 2019 04:26
control message for bug #34999
(address . control@debbugs.gnu.org)
87v9zurozb.fsf@gnu.org
severity 34999 important
L
L
Leo Prikler wrote on 24 Aug 2021 08:47
Re: Record special field abstraction leakage
bd16e5d6ff2c424945f883ac6892c5e7b0385c28.camel@student.tugraz.at
Hi Ludo,

I think I have found out why users see the thunked fields as below.
Am Dienstag, den 26.03.2019, 10:38 +0100 schrieb Ludovic Courtès:
Toggle quote (11 lines)
> The changes I made in version-control.scm and gnucash.scm in commit
> e6301fb76d0a8d931ece2e18d197e3c2cc53fc6c revealed an abstraction
> leakage
> I wasn’t aware of: there’s a pattern where users “see” that thunked
> fields are thunked:
>
> (package
> ;; …
> (inputs …)
> (arguments `(foo bar ,(inputs) …))) ;<- here ‘inputs’ is seen as
> a thunk
The issue is that for constructing the records, we let*-bind the field
names to their values before calling the constructor. In these let*-
bindings the fields are already wrapped, e.g. inputs will be bound to
the value that the record field inputs will have, not to the raw value.

I've attached a patch to fix this issue as well as a MWE to try it out.
I'm not sure about the broader semantics of this patch, though. I fear
that exposing raw values through let-binding probably eliminates the
delayed/thunked nature of said fields in some ways. WDYT?
From 1f38ff4c8b93cde533cf3d3f67358aafe9cf3dfa Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Tue, 24 Aug 2021 17:32:33 +0200
Subject: [PATCH] guix: records: let*-bind raw values, wrap them in
constructor.

This fixes the abstraction leakage mentioned in https://bugs.gnu.org/34999.

* guix/records.scm (make-syntactic-constructor)[field-bindings]: Bind to raw
value.
[field-value]: Always wrap the value.
[record-inheritance]: Wrap "inherited" values.
---
guix/records.scm | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

Toggle diff (44 lines)
diff --git a/guix/records.scm b/guix/records.scm
index ed94c83dac..074f1650c8 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -153,7 +153,10 @@ of TYPE matches the expansion-time ABI."
#`(make-struct/no-tail type
#,@(map (lambda (field index)
- (or (field-inherited-value field)
+ (or (and=>
+ (field-inherited-value field)
+ (lambda (value)
+ (wrap-field-value field value)))
(if (innate-field? field)
(wrap-field-value
field (field-default-value field))
@@ -211,8 +214,7 @@ of TYPE matches the expansion-time ABI."
(map (lambda (field+value)
(syntax-case field+value ()
((field value)
- #`(field
- #,(wrap-field-value #'field #'value)))))
+ #`(field value))))
field+value))
(syntax-case s (inherit expected ...)
@@ -224,10 +226,11 @@ of TYPE matches the expansion-time ABI."
((_ (field value) (... ...))
(let ((fields (map syntax->datum #'(field (... ...)))))
(define (field-value f)
- (or (find (lambda (x)
- (eq? f (syntax->datum x)))
- #'(field (... ...)))
- (wrap-field-value f (field-default-value f))))
+ (wrap-field-value f
+ (or (find (lambda (x)
+ (eq? f (syntax->datum x)))
+ #'(field (... ...)))
+ (field-default-value f))))
;; Pass S to make sure source location info is preserved.
(report-duplicate-field-specifier 'name s)
--
2.33.0
(use-modules (guix records)) (define-record-type* <thing> thing make-thing thing? this-thing (name thing-name (thunked)) (name2 thing-name2)) (let* ((%thing (thing (name "foo") (name2 name))) (%thing2 (thing (inherit %thing) (name "bar")))) (format #t "thing1:~% name: ~a~% name2: ~a~%~%" (thing-name %thing) (thing-name2 %thing)) (format #t "thing2:~% name: ~a~% name2: ~a~%" (thing-name %thing2) (thing-name2 %thing2)))
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 34999@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 34999
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