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

Issue 557330043: Parse inline scheme using per-expression port (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks ago by hanwenn
Modified:
2 days, 3 hours ago
Reviewers:
dak, lemzwerg, hahnjo
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Parse inline scheme using per-expression port Introduces a throw-away string port, Overlay_string_port, which makes a port out of a random section of a C string. It does not own the string, so there is no overhead in creating and discarding the ports during parse time. For GUILE v2, Overlay_string_port uses UTF-8 encoding, so UTF-8 encoded string constants within the Scheme constants are interpreted as Unicode correctly. This has several benefits: * This obviates the string port that Source_file carries along, as a result, it is no longer necessary to synchronize offsets within the parser and that port. We don't have to copy the input data in a separate byte vector anymore. In earlier versions, the offset synchronization caused a problem where LilyPond calculates offsets in the source file as bytes, while GUILE interprets the source file as UTF-8 encoded Unicode. As a result, files with Unicode before embedded scheme break completely. * We don't need to switch the encoding of the port back and forth during parsing. * We don't need to access binary-ports or %default-port-encoding This patch effectively reverts the following commits: * commit 779715fdd5a582f8882a374a9a6546501e23a84a GUILE2: Fix wide characters support in embedded SCM with guile-2.0 * commit de9f49055ca805e34e3ee537e3331b3932c58b95 GUILE2: Source_file::init_port: Keep GUILEv2 from redecoding string input Add comments that clarify how embedded scheme within #{ is #} is parsed. Rename variable in Source_file::get_counts to avoid confusing global and per-line offsets. Adds the example from 779715f as a regression test.

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : fix line/global confusion #

Patch Set 4 : rebase #

Total comments: 1

Patch Set 5 : (C) #

Patch Set 6 : set filename on port #

Total comments: 4

Patch Set 7 : configure #

Patch Set 8 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+241 lines, -120 lines) Patch
A input/regression/utf-8-scheme.ly View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
M lily/guile-init.cc View 1 2 3 1 chunk +0 lines, -8 lines 0 comments Download
M lily/include/lily-parser.hh View 1 chunk +2 lines, -0 lines 0 comments Download
A lily/include/overlay-string-port.hh View 1 2 3 4 1 chunk +166 lines, -0 lines 0 comments Download
M lily/include/source-file.hh View 1 2 3 4 5 6 7 3 chunks +2 lines, -8 lines 0 comments Download
M lily/lily-imports.cc View 1 1 chunk +0 lines, -3 lines 0 comments Download
A + lily/overlay-string-port.cc View 1 2 3 4 2 chunks +4 lines, -8 lines 0 comments Download
M lily/parse-scm.cc View 1 2 3 4 5 2 chunks +43 lines, -46 lines 0 comments Download
M lily/source-file.cc View 1 2 3 4 5 6 7 6 chunks +6 lines, -47 lines 0 comments Download
M scm/parser-ly-from-scheme.scm View 1 2 2 chunks +7 lines, -0 lines 0 comments Download

Messages

Total messages: 31
dak
Ok, that should probably have been stressed somewhat stronger before: have you checked the preexisting ...
3 weeks ago (2020-02-07 15:14:52 UTC) #1
hanwenn
On 2020/02/07 15:14:52, dak wrote: > Ok, that should probably have been stressed somewhat stronger ...
3 weeks ago (2020-02-07 15:23:10 UTC) #2
hanwenn
On 2020/02/07 15:14:52, dak wrote: > Ok, that should probably have been stressed somewhat stronger ...
3 weeks ago (2020-02-07 15:23:10 UTC) #3
dak
hanwenn@gmail.com writes: > On 2020/02/07 15:14:52, dak wrote: >> Ok, that should probably have been ...
3 weeks ago (2020-02-07 16:13:47 UTC) #4
hanwenn
On Fri, Feb 7, 2020 at 5:39 PM David Kastrup <dak@gnu.org> wrote: > > Private ...
3 weeks ago (2020-02-07 17:10:24 UTC) #5
dak
On 2020/02/07 17:10:24, hanwenn wrote: > FWIW, the 2014 commit lacks explanation and comments of ...
3 weeks ago (2020-02-07 22:17:50 UTC) #6
hanwenn
rebase
2 weeks, 5 days ago (2020-02-09 17:14:52 UTC) #7
hanwenn
David, please take another look.
2 weeks, 5 days ago (2020-02-09 17:18:19 UTC) #8
dak
On 2020/02/09 17:18:19, hanwenn wrote: > David, please take another look. First, you are aware ...
2 weeks, 5 days ago (2020-02-09 18:00:24 UTC) #9
hanwenn
On Sun, Feb 9, 2020 at 7:00 PM <dak@gnu.org> wrote: > > Now my original ...
2 weeks, 4 days ago (2020-02-10 07:27:30 UTC) #10
hanwenn
On Sun, Feb 9, 2020 at 7:00 PM <dak@gnu.org> wrote: > On 2020/02/09 17:18:19, hanwenn ...
2 weeks, 4 days ago (2020-02-10 08:41:59 UTC) #11
hanwenn
On Mon, Feb 10, 2020 at 9:41 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > Yes. See ...
2 weeks, 4 days ago (2020-02-10 09:32:37 UTC) #12
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Sun, Feb 9, 2020 at 7:00 PM <dak@gnu.org> wrote: ...
2 weeks, 4 days ago (2020-02-10 12:53:04 UTC) #13
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Mon, Feb 10, 2020 at 9:41 AM Han-Wen Nienhuys ...
2 weeks, 4 days ago (2020-02-10 13:40:11 UTC) #14
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Sun, Feb 9, 2020 at 7:00 PM <dak@gnu.org> wrote: ...
2 weeks, 4 days ago (2020-02-10 14:11:59 UTC) #15
hanwenn
On Mon, Feb 10, 2020 at 2:40 PM David Kastrup <dak@gnu.org> wrote: > > > ...
2 weeks, 4 days ago (2020-02-10 22:08:52 UTC) #16
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Mon, Feb 10, 2020 at 2:40 PM David Kastrup ...
2 weeks, 4 days ago (2020-02-10 22:21:01 UTC) #17
hanwenn
fix line/global confusion
2 weeks, 3 days ago (2020-02-11 08:43:51 UTC) #18
hanwenn
On Mon, Feb 10, 2020 at 11:21 PM David Kastrup <dak@gnu.org> wrote: > > Or ...
2 weeks, 3 days ago (2020-02-11 08:44:55 UTC) #19
hanwenn
note: this goes on top of DAK's recent patch for the .SCM file.
2 weeks, 3 days ago (2020-02-11 08:45:51 UTC) #20
hanwenn
rebase
2 weeks, 1 day ago (2020-02-13 08:27:49 UTC) #21
lemzwerg
minor nit https://codereview.appspot.com/557330043/diff/565660043/lily/include/overlay-string-port.hh File lily/include/overlay-string-port.hh (right): https://codereview.appspot.com/557330043/diff/565660043/lily/include/overlay-string-port.hh#newcode4 lily/include/overlay-string-port.hh:4: Copyright (C) 1997--2020 Han-Wen Nienhuys <hanwen@xs4all.nl> This ...
2 weeks, 1 day ago (2020-02-13 09:14:19 UTC) #22
hanwenn
(C)
2 weeks, 1 day ago (2020-02-13 10:00:19 UTC) #23
hanwenn
set filename on port
2 weeks, 1 day ago (2020-02-13 10:35:24 UTC) #24
hahnjo
https://codereview.appspot.com/557330043/diff/579310044/configure.ac File configure.ac (left): https://codereview.appspot.com/557330043/diff/579310044/configure.ac#oldcode42 configure.ac:42: GUILEv2=no Is this meant to be part of this ...
2 weeks, 1 day ago (2020-02-13 11:02:59 UTC) #25
hanwenn
configure
2 weeks, 1 day ago (2020-02-13 11:34:00 UTC) #26
hanwenn
https://codereview.appspot.com/557330043/diff/579310044/configure.ac File configure.ac (left): https://codereview.appspot.com/557330043/diff/579310044/configure.ac#oldcode42 configure.ac:42: GUILEv2=no On 2020/02/13 11:02:59, hahnjo wrote: > Is this ...
2 weeks, 1 day ago (2020-02-13 13:34:59 UTC) #27
hahnjo
On 2020/02/13 13:34:59, hanwenn wrote: > [...] > > https://codereview.appspot.com/557330043/diff/579310044/lily/include/overlay-string-port.hh > File lily/include/overlay-string-port.hh (right): > ...
2 weeks, 1 day ago (2020-02-13 13:40:28 UTC) #28
hanwenn
rebase
1 week, 3 days ago (2020-02-18 08:40:38 UTC) #29
hanwenn
this now passes tests, and is ready for submission. Anyone for a review?
1 week, 2 days ago (2020-02-19 09:48:58 UTC) #30
hanwenn
2 days, 3 hours ago (2020-02-26 22:33:57 UTC) #31
commit 483292eb80da0869d22cd773eb480c662a499e89
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Tue Feb 18 09:40:00 2020 +0100

    Parse inline scheme using per-expression port
Sign in to reply to this message.

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