Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by address@hidden)

[dak]

https://codereview.appspot.com/577410045/diff/571430047/lily/include/parse-scm.hh
File lily/include/parse-scm.hh (right):

https://codereview.appspot.com/577410045/diff/571430047/lily/include/parse-scm.hh#newcode30
lily/include/parse-scm.hh:30: SCM parse_embedded_scheme (Input *i, bool
safe, Lily_parser *parser);
On 2020/01/28 22:06:32, hanwenn wrote:
> On 2020/01/27 13:39:31, Dan Eble wrote:
> > Changing Input& to Input* is more than cosmetic.  Input& requires an
object,
> but
> > Input* admits a nullptr.  I'm concerned that I don't see that any
checks have
> > been added before the pointer is dereferenced.
> 
> This code stores a reference, which is completely out of style in
LilyPond.

It's pretty untypical, not least because unsmobbing works on pointers. 
That being said, Input is likely the least coherently treated data
structure in that regard if I remember correctly, quite often getting
passed by copy (as seen in the signature of evaluate_embedded_scheme). 
Part of the reason for this use may be that for Scheme protection to set
in, you need to actually smobify the structure and then keep an SCM
value around in the stack somewhere which may prove inconvenient. 
Having a local copy and passing a reference around is quite more static
regarding the life times.

Things go as far as Input() creating a "null input" without location
data, so the value "no input" is not actually represented by using a
null pointer.

All that peculiarity of Input was not invented by me but originally
there.  Part of the reason may be that parser.yy needs a fixed data type
for its @$ and similar location data, so the C structure interface
inherent there might have been partly responsible for this somewhat
peculiar usage.

I don't have a particular opinion on this usage myself but think that
the current use of a reference might have been done by me trying to keep
with the general spirit of how Input is getting used without losing
sight of C++.

Not necessarily successfully so, but I doubt there were a lot of
reviewers advising me at the time.

https://codereview.appspot.com/577410045/