Re: Layout of ‘define-configuration’ records

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>> +               (%location #,(id #'stem #'stem #'-location)
>>> +                          (default (and=> (current-source-location)
>>> +                                          source-properties->location))
>>> +                          (innate)))
>>
>> Moving the field last is problematic as we’ve seen for any user that
>> uses ‘match’ on records—something that’s not recommended but still used
>> a lot.
>
> Yep.  I had that on mind when I made the change, though I still missed a
> few occurrences.

[...]

> I wanted match on define-configuration'd fields to be
> backward-compatible with fields migrated from define-record-type*, so
> that they such change can be made without worrying breakages.

That had the opposite effect: it introduced breakage precisely because
existing uses of ‘match’ on records need to be verified manually, one by
one.

That led me to improve ‘match-record’, to recommend it in the manual,
and do “convert” some uses of ‘match’ to ‘match-record’:

  https://issues.guix.gnu.org/59390

That’s a good outcome, but I’d prefer not feeling this kind of pressure.

> I initially got tricked by that discrepancy and it took me quite some
> time hunting down a cryptic backtrace when authoring the new mcron
> configuration records.

I see.  However, this is a wide-ranging change, so I think this should
have been discussed separately from the mcron changes.  I find it
important to take time for review and discussion for such changes.

>> One last thing: placing ‘%location’ first can let us implement:
>>
>>   (define (configuration-location config)
>>     (struct-ref config 0))
>
> Would this have worked?
>
> scheme@(gnu services mcron)> (define config (mcron-configuration))
> scheme@(gnu services mcron)> (struct-ref 0 config)

You got the order wrong.  :-)

> All in all, I think that's a rather small change that got our internal
> implementation of both type of records in sync between
> define-configuration and define-record-type*, that should pave the way
> for migrating more of the later to the former without risking breaking
> something, going forward.

Fundamentally, the layout of record types should not be visible to
users.  That it is visible via ‘match’ is the problem, and the solution
is not to tweak record type layout but instead tp make sure ‘match’ uses
on records vanish.

I hope that makes sense!

> scheme@(gnu services mcron)> ,use (oop goops)

Speaking of which: there was a conscious decision to not use GOOPS in
Guix from day one.  Perhaps some day we’ll want to collectively question
that, but let’s make sure we don’t add that dependency on a whim.

For example:

  class-of -> struct-vtable
  class-name -> record-type-name

See commit 50c17ddd9e2983d71c125d89b422fd20fca476e1 for an example.

Thanks,
Ludo’.