Re: parser.yy: allow postevents in function arguments in general (issue 5339043)

[dak]

Reviewers: J_lowe,

Message:
On 2011/11/05 11:54:16, J_lowe wrote:
> Patch fails to apply to current master

Hardly surprising.  This is why it says right at the top "This change is
on top of the dev/syntax branch still in review." and why its related
issue 2018 is blocked on issue 2001.

I'd appreciate testing, but would not lose sleep over it.  At the
current point of time, comments might be more important.

Description:
parser.yy: allow postevents in function arguments in general

This change is on top of the dev/syntax branch still in review.  It
removes the "polymorphism" of music functions used explicitly as events.

The previous behavior was such that trailing arguments of kind ly:music?
were parsed as postevents instead of music, so that \tweak had a chance
to be implemented as a music function.  This concept has become
increasingly fishy when ly:music? predicates where treated just like any
old Scheme predicate.  Event functions (and music functions used as
events) have not inherited the new power of music function argument
lists.  Doing that with something akin to the previous semantics would
have required a large amount of code duplication since one can't easily
replicate a complex _recursive_ grammar while exchanging a core element.

This patch sidesteps the issue by just allowing both postevents and
normal music as function arguments for both music and event functions.

It passes the regtests, but there are consequences:

a) a music function can't figure out whether it is used in a postevent
or a music event setting by looking at the type of music argument  it
receives.  That type is orthogonal from how its return value  gets used.

b) a music function might receive postevents where previously syntax
errors occured.

To be fair: most music functions were not bothering checking whether
they might be employed in postevent settings anyway, so it is not like
this adds a fundamentally new class of surprises.

I am still putting it out for separate review.

Please review this at http://codereview.appspot.com/5339043/

Affected files:
  M lily/parser.yy


Index: lily/parser.yy
diff --git a/lily/parser.yy b/lily/parser.yy
index  
63a0bd11e00381e77c360d725d972775bebc4ee9..bb98b71b2ba60ef8a698dd60e0351c535c567bb0  
100644
--- a/lily/parser.yy
+++ b/lily/parser.yy
@@ -464,7 +464,6 @@ If we give names, Bison complains.
 %type <scm> embedded_scm_bare_arg
 %type <scm> embedded_scm_closed
 %type <scm> embedded_scm_chord_body
-%type <scm> embedded_scm_event
 %type <scm> event_function_event
 %type <scm> figure_list
 %type <scm> figure_spec
@@ -507,7 +506,6 @@ If we give names, Bison complains.
 %type <scm> mode_changing_head_with_context
 %type <scm> multiplied_duration
 %type <scm> music_function_event
-%type <scm> music_function_event_arglist
 %type <scm> music_function_chord_body
 %type <scm> music_function_chord_body_arglist
 %type <scm> new_chord
@@ -1279,6 +1277,10 @@ function_arglist_nonbackup:
        {
                $$ = check_scheme_arg (PARSER, @4, $1, $4, $3, $2);
        }
+       | EXPECT_OPTIONAL EXPECT_SCM function_arglist_closed post_event
+       {
+               $$ = check_scheme_arg (PARSER, @4, $1, $4, $3, $2);
+       }
        ;


@@ -1303,6 +1305,16 @@ function_arglist_backup:
                        MYBACKUP (SCM_IDENTIFIER, $4, @4);
                }
        }
+       | EXPECT_OPTIONAL EXPECT_SCM function_arglist_closed_keep post_event
+       {
+               if (scm_is_true (scm_call_1 ($2, $4)))
+               {
+                       $$ = scm_cons ($4, $3);
+               } else {
+                       $$ = scm_cons (loc_on_music (@3, $1), $3);
+                       MYBACKUP (EVENT_IDENTIFIER, $4, @4);
+               }
+       }
        | EXPECT_OPTIONAL EXPECT_SCM function_arglist_keep lyric_element
        {
                // There is no point interpreting a lyrics string as
@@ -1414,6 +1426,11 @@ function_arglist_common:
                $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
                                       $3, $2, $1);
        }
+       | EXPECT_SCM function_arglist_closed_optional post_event
+       {
+               $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
+                                      $3, $2, $1);
+       }
        | function_arglist_common_lyric
        ;

@@ -1474,6 +1491,11 @@ function_arglist_closed_common:
                $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
                                       $3, $2, $1);
        }
+       | EXPECT_SCM function_arglist_closed_optional post_event
+       {
+               $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
+                                      $3, $2, $1);
+       }
        | EXPECT_SCM function_arglist_closed_optional FRACTION
        {
                $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
@@ -2016,41 +2038,20 @@ music_function_chord_body:
        }
        ;

-/* We could accept a closed music argument before the post events
- * indicated by a trailing argument list.  For symmetry with chord
- * bodies and in order to avoid too tricky and complex behavior, we
- * refrain from doing so.
- */
-music_function_event_arglist:
-       function_arglist_bare
-       | EXPECT_SCM music_function_event_arglist embedded_scm_event
-       {
-               $$ = check_scheme_arg (PARSER, @3, SCM_UNDEFINED,
-                                      $3, $2, $1);
-       }
-       ;
-
-embedded_scm_event:
-       embedded_scm_bare_arg
-       | SCM_FUNCTION music_function_event_arglist {
-               $$ = MAKE_SYNTAX ("music-function", @$,
-                                        $1, $2);
-       }
-       | bare_number
-       | fraction
-       | lyric_element
-       | post_event
-       ;
+// Event functions may only take closed arglists, otherwise it would
+// not be clear whether a following postevent should be associated
+// with the last argument of the event function or with the expression
+// for which the function call acts itself as event.

 music_function_event:
-       MUSIC_FUNCTION music_function_event_arglist {
+       MUSIC_FUNCTION function_arglist_closed {
                $$ = MAKE_SYNTAX ("music-function", @$,
                                         $1, $2);
        }
        ;

 event_function_event:
-       EVENT_FUNCTION music_function_event_arglist {
+       EVENT_FUNCTION function_arglist_closed {
                $$ = MAKE_SYNTAX ("music-function", @$,
                                         $1, $2);
        }
@@ -2322,7 +2323,7 @@ gen_text_def:
                t->set_property ("text", $1);
                $$ = t->unprotect ();
        }
-       | string {
+       | simple_string {
                Music *t = MY_MAKE_MUSIC ("TextScriptEvent", @$);
                t->set_property ("text",
                        make_simple_markup ($1));