GNU bug report logs - #50286
[RFC PATCH] Let 'package-location' returns location of surrounding 'let'.

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: iskarian <at> mgsn.dev, 50286 <at> debbugs.gnu.org
Subject: [bug#50286] [RFC PATCH] Let 'package-location' returns location of surrounding 'let'.
Date: Mon, 06 Sep 2021 12:07:04 +0200

Hello,

Maxime Devos <maximedevos <at> telenet.be> skribis:

> These three patches allows (guix upstream) to replace the values in the surrounding 'let'
> form, if any.  It's important for constructs like:
>
> (define-public gnash
>    (let ((version "0.8.11")
>          (commit "583ccbc1275c7701dc4843ec12142ff86bb305b4")
>          (revision "0"))
>      (package
>        (name "gnash")
>        (version (git-version version revision commit))
>        (source (git-reference
>                  (url "https://example.org")
>                  (commit commit)))
>        [...])))
>
> such that it can update the version, commit, revision. (Currently only the
> version will be updatable, but see <https://issues.guix.gnu.org/50072#0>
> and <https://issues.guix.gnu.org/50072#9> for work on making 'commit' updatable).

This is smart!

I wonder if we’re going overboard, though.  Intuitively, I would rather
leave ‘location’ fields dumb, and instead add editing features to do
things like getting the location of the parent sexp.  It does add some
overhead, but it also makes things more explicit and preserves
separation of concern.  (Also, in ‘core-updates-frozen’,
‘go-to-location’ uses a location cache that makes it less expensive than
on ‘master’.)  But yeah, it’s trickier…

Hmm, thinking out loud, what about this: use the same trick as you did,
but replace ‘define-public’ instead of ‘let’ & co., so as to be less
intrusive.

  (define-syntax-parameter current-definition-location
    (identifier-syntax #f))

  (define-syntax define-public*
    (syntax-rules ()
      ((_ prototype body)
       (define-public prototype
         (syntax-parameterize ((current-definition-location
                                (identifier-syntax (current-source-location))))
           body)))))

Since there’s code that assumes ‘package-location’ returns the location
of the (package …) sexp, we could add a ‘definition-location’ field in
<package>, defaulting to ‘current-definition-location’, or tweak
‘location’ to include both.

WDYT?

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.