Re: T3154 work - question on init code, ly_book_scheme initialization passing Lexer instance as value for scheme (parser), why is this?

[Ian Hulin]

On 10/07/13 23:39, David Kastrup wrote:
> Ian Hulin <address@hidden> writes:
> 
>> I don't understand why when I set a breakpoint at ly_book_scheme
>> I get a parameter passed which should be the value of scheme
>> (parser) coming through as a Lexer type.
> 
> I don't understand that sentence at all.
> 
Sorry for the gibberish.  I've added an extra param to some calls to
Scheme procedures in lily-library.scm to pass (parser) to
ly:book-process.  They are coming through to the code wrong.  I think
it's due to the signature mistake in LY_DEFINE as you explain below.
Another D'Oh! moment...

>> This is part of work for issue 3154.
>> 
>> But here's a summary of what's I've been doing and some gdb
>> ouptut.
>> 
>> Once upon a time I thought getting alternately named and/or
>> suffixed would be a nice small-scale fix I could get done and
>> dusted in time to go into V2.18.  As David K would say, colour me
>> three shades of older, sadder and wiser.  It's involved changing
>> quite a few files.
>> 
>> The basic design is to track the incoming \midi {} blocks,
>> reference them in an enhanced Performance:: object and then haul
>> these out and scan them for file-name="blah" and
>> file-suffix="thingy" clauses before writing out the midi files in
>> the procedure in scm/midi.scm.  The idea here is to use
>> output_def_lookup to get hold of the property values.
> 
> The question is whether it even makes sense trying to go through
> \midi or whether one should actually use the (somewhat
> ill-fittingly named) \paper block instead.
> 
>> I've changed the LY_DEFINE definition for ly_book_process to
>> 
>> LY_DEFINE (ly_book_process, "ly:book-process", 4, 0, 1, (SCM
>> book_smob, SCM default_paper, SCM default_layout, SCM output, SCM
>> parser_ref),
>> 
>> This is so that some user who calls ly:book_process with the old 
>> signature from scheme doesn't get their code broken by this
>> change.
> 
> Why not 4, 1, 0 instead?
> 
>> 
>> Simple testing with GDB shows that if the procedure is called
>> with just the four required parameters, the C++ sees parser_ref
>> as being SCM_EOL (alias '()), and so I should be able to handle
>> this.
>> 
>> I've guarded the LY_ASSERT_SMOB call with
>> 
>> /* UGH - working round that initialization may pass an instance
>> of the lexer in parser_ref. */ Lily_parser * my_parser; 
>> //Lily_lexer * my_lexer; my_parser  = unsmob_lily_parser (
>> parser_ref ); // 0 if parser_ref == SCM_EOL if (parser_ref !=
>> SCM_EOL && !parser_ref) { LY_ASSERT_SMOB (Lily_parser,
>> parser_ref, 4) }
>> 
>> 
>> However, I get this from an ly:parse-file call in the sandbox
>> with one of the regression test sources
>> (input/regression/midi/key-initial.ly) as the input: I've set a
>> break in gdb at ly_book_process and printed the scheme value 
>> passed in parser_ref (the new, optional parameter)
>> 
>> (gdb) help ps ps - print variable packed in a SCM structure 
>> Interpret $arg0 as a scheme string by calling scm_display and
>> scm_newline to output them to stdout (gdb) ps parser_ref 
>> (#<Lily_parser #<Lily_lexer (#<module b56588e0>) > >) $41 = void 
>> (gdb)
>> 
>> Can anyone help explain how/why I get this instead of a simple 
>> #<Lily_parser>, please?
> 
> Because you were asking for a REST argument (which is a list of
> all remaining arguments) rather than an OPTIONAL argument (which is
> either SCM_UNDEFINED or given).  The counts are MANDATORY,
> OPTIONAL, REST, so you probably wanted 4,1,0 rather than 4,0,1 .
> 
D'oh!! Thaks for spotting that.

Cheers,

Ian