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

["Ludovic Courtès" <ludo@gnu.org>]

Hello,

Maxime Devos <maximedevos@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’.