Re: Some bugs (???) in lexer.ll, parser.yy of Lilypond 2.9.9

[Erik Sandberg]

On Friday 07 July 2006 17:56, Angelo Contardi wrote:
> Hello Han-Wen,
>
> i'm not a musician (though i like music) but a C programmer with some
> experience in flex/yacc parser (just C, not C++).
> During this week i have take a look at lexer.ll (mostly) and parser.yy
> (just few time) and i think there are some errors:
>
> lexer.ll:
> ---------
>
> 1) The TEX syntax include both PUNCT and ACCENT so in the LYRICS may
>    include ' and ` both "escaped" (ACCENT) and "non escaped" (PUNCT).
>    To solve this problem probably you should define:
>
>       PUNCT [?!:]
>
>    And allow only escaped version of ` and ', like in C strings.
>    Note that PUNCT and ACCENT appear only in TEX, that appears only in
> LYRICS.

Why is it a problem?

> 2) Not a bug, but a possible simplification. The "semantic actions" in the
>    status <quote> and <lyric_quote> are the same, except for the token
>    returned (respectively STRING and LYRICS_STRING). So both status may be
>    re-wrote in a compact form:

OK.

>    But my question is: do you really need to distinguish between STRING and
>    LYRICS_STRING? 

Yes, it by keeping them separate, some shift/reduce conflicts are eliminated 
from the parser.

>    In any case you can "group" all kind of "start quote" in this compact
> way:
>
>    #define start_quote()              \
>       if (YY_START==lyric)            \ // start_lyric_quote() was called
> Just yy_push_state (lyric_quote); \ // in <lyric> status
>       else                            \
>          yy_push_state (quote);       \
>       yylval.string = new string

Why can't we just use separate defines for start_lyric_quote and start_quote, 
and only group all start_quotes together?

> parser.yy:
> ----------

Changes look sensible to me, except some bugs I found and fixed. Also, I'd 
prefer to give < and << the names ANGLE_OPEN nad DOUBLE_ANGLE_OPEN (they are 
used for non-chord situations as well).

I also fixed some other problems in your code, but ignored the TEX stuff 
because I don't understand it. Attached is a patch which works well as far as 
I have tested. Han-Wen, may I apply?

-- 
Erik

angelo.diff
Description: Text Data