Package: guix-patches;
Reported by: Philip McGrath <philip <at> philipmcgrath.com>
Date: Tue, 8 Feb 2022 15:14:01 UTC
Severity: normal
Tags: patch
Merged with 53997
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
Message #117 received at 53878 <at> debbugs.gnu.org (full text, mbox):
From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> To: Philip McGrath <philip <at> philipmcgrath.com>, 53878 <at> debbugs.gnu.org Subject: Re: [PATCH 04/11] gnu: chez-and-racket-bootstrap: Add utilities for Chez machine types. Date: Thu, 17 Feb 2022 08:24:24 +0100
Hi,
Am Mittwoch, dem 16.02.2022 um 17:54 -0500 schrieb Philip McGrath:
> > > +See @code{chez-machine->nix-system} for more details about
> > > acceptable values
> > > +for MACH."
> > > + (let ((mach (chez-machine->unthreaded mach)))
> > > + (cond
> > > + ((string-prefix? "arm64" mach)
> > > + 'no-support)
> > > + ((string-prefix? "arm32" mach)
> > > + (if (string-suffix? "le" mach)
> > > + 'no-threads
> > > + 'no-support))
> > > + ((string-prefix? "ppc32" mach)
> > > + (if (string-suffix? "le" mach)
> > > + #f
> > > + 'no-support))
> > > + (else
> > > + #f))))
> > -> is a conversion operator, not an "accessor".
>
> I thought of this as a conversion more than an accessor.
Conversion follows the lines of "is (roughly) a", whereas this is a
"has a" relation, which would imply accessing things.
> > "upstream-restriction" sounds rather negative, I'd rather have
> > (chez-machine-features), which yields #f if the machine is
> > unsupported and a (possibly empty) list of features otherwise, such
> > as '(threads).
> >
> > I'm also not quite sure what the point is behind using chez
> > machines here. Why not simply test the systems with the predicates
> > we already have, i.e. target-arm64?, target-arm32?, target-linux?,
> > target-ppc32?,
> > ...
>
> I agree that the name of this procedure should avoid sounding
> negative: 'restriction' was the best I'd come up with so far. I
> think I like 'chez-machine-features' somewhat better, but with a few
> reservations.
>
> Using predicates like 'target-arm32?' makes some sense overall, and
> I'll try it. One subtly (sic) is that systems supported by neither
> upstream nor Racket are currently handled by `and=>` in clients.
>
> Also, I guess it's odd to have this function operate on Chez Scheme
> machine types anyway, since a machine type specifies threaded or
> unthreaded.
Not sure how to deal with these reservations or what they'd imply, so
following along.
> I'm currently thinking something like:
>
> --8<---------------cut here---------------start------------->8---
> (define* (chez-upstream-features-for-system #:optional
> (system (or
> (%current-target-system)
>
> (%current-system))))
> (cond
> ((not (nix-system->chez-machine system))
> #f)
> ((target-arm64? system)
> #f)
> ((target-arm32? system)
> (and (target-linux? system)
> '()))
> ((target-ppc32? system)
> (and (target-linux? system)
> '(threads)))
> (else
> '(threads))))
> --8<---------------cut here---------------end--------------->8---
Haven't tested this, but LGTM.
> Alternatively, there could be an argument called something like
> "variant", where the default would be 'upstream or maybe #f, and
> supplying 'racket would always return '(threads). (Racket CS requires
> the threaded version of Chez Scheme, so the chez-scheme-for-racket
> has consistently added thread support immediately upon adding any new
> target.)
I don't think delegating to racket this soon is a good idea.
> > And as a minor pet peeve, you ought to spell out machine.
> >
>
> Ok, will do.
>
> >
> > > +(define* (nix-system->chez-machine #:optional (system (%current-
> > > system))
>
> This should have been (or (%current-target-system) (%current-
> system)).
>
> > > + #:key (threads? 'always))
> > > + "Return the Chez Scheme machine type corresponding to the Nix
> > > system
> > > +identifier SYSTEM, or @code{#f} if the translation of SYSTEM to
> > > a
> > > Chez Scheme
> > > +machine type is undefined.
> > > +
> > > +When THREADS? is @code{'always} (the default), the threaded
> > > variant
> > > of the
> > > +machine type will be returned: note that the package returned by
> > > +@code{chez-scheme-for-system} will always support native
> > > threads.
> > > When
> > > +THREADS? is @code{#f}, the unthreaded machine type will be
> > > returned. If
> > > +THREADS? is @code{'upstream} (the default), the threaded variant
> > > of
> > > the
> > > +machine type will be returned if and only if it is supported by
> > > upstream Chez
> > > +Scheme (see @code{chez-machine->upstream-restriction}). If
> > > THREADS?
> > > is any
> > > +other value, an exception is raised."
> > What's the point in having THREADS? 'always? In any case, assuming
> > chez-machine-features is to be exported, this can easily be checked
> > --
> > even if not, we can add the check internally by writing
> > #:key (threads? (chez-supports-threads? system))
> >
> > > + (let* ((hyphen (string-index system #\-))
> > > + (nix-arch (substring system 0 hyphen))
> > > + (nix-os (substring system (+ 1 hyphen)))
> > > + (chez-arch (assoc-ref %nix-arch-to-chez-alist nix-
> > > arch))
> > > + (chez-os (assoc-ref %nix-os-to-chez-alist nix-os))
> > > + (mach (and chez-arch chez-os (string-append chez-arch
> > > chez-
> > > os))))
> > This series of let-bindings should probably be done in a separate
> > function called nix-system->chez-machine.
>
> On the one hand, the result of 'chez-scheme-for-system' will always
> support threads, so being threaded by default is convenient. On the
> other hand, it looks like I've reduced the use of this function (by
> searching for files rather than coding in the machine type directory)
> enough that it doesn't have a big impact either way. It will be more
> important when we support cross-compilation, and I think we should
> keep these functions internal and defer as much as possible of their
> API design until then.
>
> For now, I think I'll say that it's unspecified whether the result is
> a threaded or unthreaded machine type and add
> `chez-machine->{un,}threaded` as needed.
I think you can keep (chez-supports-threads? system) hidden even when
using it to instantiate defaults. I currently don't see the relation
between chez-scheme-for-system and nix-system->chez-machine, so if
that's relevant, you'd have to clarify.
Cheers
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.