Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(760)

Issue 553480044: Use define-syntax for define-session[-public] (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by hanwenn
Modified:
4 years, 1 month ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Use define-syntax for define-session[-public] defmacro with inner functions are not supported in GUILE 2.x as the inner functions only exist at expansion time, and not at runtime. This gets rid of the error unhandled constant #<procedure ... > during autocompile

Patch Set 1 #

Patch Set 2 : Guile 1.8 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -21 lines) Patch
M scm/lily.scm View 1 4 chunks +26 lines, -21 lines 5 comments Download

Messages

Total messages: 13
hanwenn
Guile 1.8
4 years, 1 month ago (2020-01-31 18:24:03 UTC) #1
hanwenn
David?
4 years, 1 month ago (2020-02-01 21:15:56 UTC) #2
dak
https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm File scm/lily.scm (right): https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm#newcode111 scm/lily.scm:111: "This defines a variable @var{name} with the starting value ...
4 years, 1 month ago (2020-02-01 21:37:50 UTC) #3
hanwenn
https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm File scm/lily.scm (right): https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm#newcode111 scm/lily.scm:111: "This defines a variable @var{name} with the starting value ...
4 years, 1 month ago (2020-02-02 14:01:33 UTC) #4
dak
On 2020/02/02 14:01:33, hanwenn wrote: > https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm > File scm/lily.scm (right): > > https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm#newcode111 > ...
4 years, 1 month ago (2020-02-02 15:28:44 UTC) #5
hanwenn
On Sun, Feb 2, 2020 at 4:28 PM <dak@gnu.org> wrote: > > Please see Taylan's ...
4 years, 1 month ago (2020-02-02 20:04:55 UTC) #6
hanwenn
On 2020/02/02 15:28:44, dak wrote: > On 2020/02/02 14:01:33, hanwenn wrote: > > https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm > ...
4 years, 1 month ago (2020-02-03 09:49:32 UTC) #7
dak
On 2020/02/03 09:49:32, hanwenn wrote: > On 2020/02/02 15:28:44, dak wrote: > > That's what ...
4 years, 1 month ago (2020-02-03 12:11:08 UTC) #8
dak
On 2020/02/03 12:11:08, dak wrote: > On 2020/02/03 09:49:32, hanwenn wrote: > > On 2020/02/02 ...
4 years, 1 month ago (2020-02-03 15:32:23 UTC) #9
hanwenn
On 2020/02/03 12:11:08, dak wrote: > > What is wrong with the code I propose ...
4 years, 1 month ago (2020-02-04 08:56:49 UTC) #10
dak
On 2020/02/04 08:56:49, hanwenn wrote: > On 2020/02/03 12:11:08, dak wrote: > > > What ...
4 years, 1 month ago (2020-02-04 10:42:51 UTC) #11
dak
NLGTM This patch just punts on using an internal function, and that could be equally ...
4 years, 1 month ago (2020-02-04 21:15:44 UTC) #12
hanwenn
4 years, 1 month ago (2020-02-04 22:09:45 UTC) #13
On 2020/02/04 21:15:44, dak wrote:
> NLGTM
> 
> This patch just punts on using an internal function, and that could be equally
> well be done without involving syntax-case.
> 
> An alternative proposal that just relies on a single external symbol in order
to
> achieve the original design is given as
> 
> Tracker issue: 5735 (https://sourceforge.net/p/testlilyissues/issues/5735/)
> Rietveld issue: 549510043 (https://codereview.appspot.com/549510043)
> Issue description:
>   Rewrite define-session and define-session-public macros  The byte
>   compiler of Guile-2.x is not able to compile anonymous
>   functions/closures into constants.  Rewriting the macros not to rely
>   on such constants by moving the local functions into their own
>   function definitions is the easiest expedient.
> 
> https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm
> File scm/lily.scm (right):
> 
>
https://codereview.appspot.com/553480044/diff/557280043/scm/lily.scm#newcode135
> scm/lily.scm:135: (acons (quote name) (make-session-variable (quote name)
value)
> lilypond-exports))
> This is actually a purely textual replacement rather than anything more
complex
> or hygienic.  As such it does nothing that cannot equally well be achieved
using
> defmacro: the problematic issue with the byte compiler was the internal
function
> that was used for _avoiding_ a wholesale textual replacement.
> 
> So drawing in a module that is known to be buggy and unmaintained in Guile-1.8
> does not even serve a purpose here.

closing in favor of https://codereview.appspot.com/549510043/ - you have my
permission to skip countdown :-)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b