Re: Make music functions callable from Scheme (issue 244840043 by address@hidden)

[k-ohara5a5a]

It bothered me that I said 'LGTM' without figuring out the logic.  I
don't understand it.


https://codereview.appspot.com/244840043/diff/80001/lily/music-function.cc
File lily/music-function.cc (right):

https://codereview.appspot.com/244840043/diff/80001/lily/music-function.cc#newcode63
lily/music-function.cc:63: bool skipping = false;
The 'skipping' flag and the early continues are hard to follow.  We want
to keep this logic synchronized with the duplicate functionality in the
parser, so either now or later it would be good to make the logic more
clear.

What would be clear for me is a boolean remembering whether the argument
currently being matched is optional:

for ( ; signature; ++signature)

  if pair(car(signature))
    pred = caar(signature)
    default = cadr(signature) or #f
    optional = true
  else
    pred = car(signature)
    optional = false
    if !input_arg
      error "too few args"
      return

  if type-predicate( input_arg )
    arg = input_arg++
  elseif optional
    arg = default_arg
    if *unspecified* == input_arg
      input_arg++
  else
    error "wrong argument type"
    return

  append arg to argslist

endfor

if input_arg
  warn "too many args"

https://codereview.appspot.com/244840043/diff/80001/lily/music-function.cc#newcode83
lily/music-function.cc:83: skipping = true;
Here we failed to type-match an optional argument, and will fill the
slot with the default value on line 88, so why set the 'skipping' flag ?
 If there are two optional parameters in a row, the 'skipping' flag
would seem to force the next entry in the signature to be filled with
its default value as well.

https://codereview.appspot.com/244840043/diff/80001/lily/music-function.cc#newcode88
lily/music-function.cc:88: args = scm_cons (with_loc (scm_cdr (pred),
location), args);
For an optional argument with no default value, it seems we would want
#f, but scm_cdr will return the empty list.

https://codereview.appspot.com/244840043/