Re: Wrong type in part-combiner.scm

[Aaron Hill]

On 2020-09-22 5:54 am, David Kastrup wrote:
> The whole code in the find-non-empty loop, apart from this error, is
> garbage.  Here is a bit more code and analysis:
>
> (define-public (add-quotable name mus)
>   (let* ((tab (eval 'musicQuotes (current-module)))
>          (voicename (get-next-unique-voice-name))
>          ;; recording-group-emulate returns an assoc list (reversed!), 
> so
>          ;; hand it a proper unique context name and extract that key:
>          (ctx-spec (context-spec-music mus 'Voice voicename))
>          (listener (ly:parser-lookup 'partCombineListener))
>          (context-list (reverse (recording-group-emulate ctx-spec 
> listener)))
>          (raw-voice (assoc voicename context-list))
>          (quote-contents (if (pair? raw-voice) (cdr raw-voice) '())))
>
>     ;; If the context-specced quoted music does not contain anything, 
> try to
>     ;; use the first child, i.e. the next in context-list after 
> voicename
>     ;; That's the case e.g. for \addQuote "x" \relative c \new Voice 
> {...}
>     (if (null? quote-contents)
>         (let find-non-empty ((current-tail (member raw-voice 
> context-list)))
>           ;; if voice has contents, use them, otherwise check next ctx
>           (cond ((null? current-tail) #f)
>                 ((and (pair? (car current-tail))
>                       (pair? (cdar current-tail)))
>                  (set! quote-contents (cdar current-tail)))
>                 (else (find-non-empty (cdr current-tail))))))
>
>     (if (not (null? quote-contents))
>         (hash-set! tab name (list->vector (reverse! quote-contents 
> '())))
>         (ly:music-warning mus (ly:format (_ "quoted music `~a' is
> empty") name)))))
>
> So raw-voice is an assoc into context-list.  This assoc is then 
> searched
> for in the find-non-empty loop.  For each found instance, a test is
> made.  If that test is not successful, the search is repeated for more
> instances.  However, since all found instances are _guaranteed_ to be
> the same (since assoc is only called once, and then member searches for
> the same key/value pair rather than just the same key), the test is
> guaranteed to deliver the same result for each found instance.
>
> This is just crap.  Presumably the idea was to search for the same key
> but potentially different values?

I don't think this code matches multiple times.  The call to member only 
occurs upon the first invocation of find-non-empty.  Each subsequent 
time, the current-tail parameter gets the value of (cdr current-tail) 
which is simply iterating the list until running out of items.

The problem with raw-voice being #f, though, is an issue.  It is highly 
unlikely to be found in the context-list, so member produces another #f 
that ultimately leads to the failing (cdr #f).  At first, I would wonder 
if raw-voice should never be #f but then I see code checking (pair? 
raw-voice), so it must be valid edge case.

What I cannot tell is whether the entire context-list is to be searched 
when raw-voice is #f or whether the find-non-empty logic should be 
skipped altogether.

So, if we want to fall back to searching the entire list, do this:

;;;;
  (let find-non-empty ((current-tail (or (member raw-voice context-list)
                                         context-list)))
;;;;

Or if we want to skip the searching, do this:

;;;;
  (if (and (null? quote-contents) (pair? raw-voice))
;;;;

Absolutely no idea which is correct.  Sorry.  :/


-- Aaron Hill