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/