When cloning a lexer/parser, don't copy the scopes.

When cloning a lexer/parser, don't copy the scopes.

[Trevor Daniels, 2012-08-10 06:58:57]

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc
File lily/lily-parser.cc (right):

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc#newcode160
lily/lily-parser.cc:160: // TODO: use $parser
Either this comment should be removed, or the
earlier one should be left in.  Sorry, I don't
understand enough of this to know which.

[dak, 2012-08-10 07:13:58]

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc
File lily/lily-parser.cc (right):

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc#newcode160
lily/lily-parser.cc:160: // TODO: use $parser
On 2012/08/10 06:58:57, Trevor Daniels wrote:
> Either this comment should be removed, or the
> earlier one should be left in.  Sorry, I don't
> understand enough of this to know which.

There is not enough context to figure out what the author of the comment was
aiming for.  That's why I took the other comment out.  Will remove this one as
well but not submit a separate review.

[hanwenn, 2012-08-10 16:14:55]

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc
File lily/lily-parser.cc (right):

http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc#newcode160
lily/lily-parser.cc:160: // TODO: use $parser
On 2012/08/10 07:13:58, dak wrote:
> On 2012/08/10 06:58:57, Trevor Daniels wrote:
> > Either this comment should be removed, or the
> > earlier one should be left in.  Sorry, I don't
> > understand enough of this to know which.
> 
> There is not enough context to figure out what the author of the comment was
> aiming for.  That's why I took the other comment out.  Will remove this one as
> well but not submit a separate review.

If memory serves, this is about changing the name for this variable to indicate
it is internal.

I'm also not sure how badly things will break (eg. in safe mode), if someone
does 

 parser = "foo"

[dak, 2012-08-10 16:16:47]

On 2012/08/10 16:14:55, hanwenn wrote:
> http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc
> File lily/lily-parser.cc (right):
> 
> http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc#newcode160
> lily/lily-parser.cc:160: // TODO: use $parser
> On 2012/08/10 07:13:58, dak wrote:
> > On 2012/08/10 06:58:57, Trevor Daniels wrote:
> > > Either this comment should be removed, or the
> > > earlier one should be left in.  Sorry, I don't
> > > understand enough of this to know which.
> > 
> > There is not enough context to figure out what the author of the comment was
> > aiming for.  That's why I took the other comment out.  Will remove this one
as
> > well but not submit a separate review.
> 
> If memory serves, this is about changing the name for this variable to
indicate
> it is internal.
> 
> I'm also not sure how badly things will break (eg. in safe mode), if someone
> does 
> 
>  parser = "foo"

Nobody keeps you from writing
"$parser" = "foo"

and #{ ... #} looks for "parser", and you don't want to change the name of the
first argument of music functions.

[hanwenn, 2012-08-10 16:20:30]

On 2012/08/10 16:16:47, dak wrote:
> On 2012/08/10 16:14:55, hanwenn wrote:
> > http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc
> > File lily/lily-parser.cc (right):
> > 
> >
http://codereview.appspot.com/6460064/diff/4001/lily/lily-parser.cc#newcode160
> > lily/lily-parser.cc:160: // TODO: use $parser
> > On 2012/08/10 07:13:58, dak wrote:
> > > On 2012/08/10 06:58:57, Trevor Daniels wrote:
> > > > Either this comment should be removed, or the
> > > > earlier one should be left in.  Sorry, I don't
> > > > understand enough of this to know which.
> > > 
> > > There is not enough context to figure out what the author of the comment
was
> > > aiming for.  That's why I took the other comment out.  Will remove this
one
> as
> > > well but not submit a separate review.
> > 
> > If memory serves, this is about changing the name for this variable to
> indicate
> > it is internal.
> > 
> > I'm also not sure how badly things will break (eg. in safe mode), if someone
> > does 
> > 
> >  parser = "foo"
> 
> Nobody keeps you from writing
> "$parser" = "foo"
> 
> and #{ ... #} looks for "parser", and you don't want to change the name of the
> first argument of music functions.

the first name of the music function argument is irrelevant, as it locally
scoped. Other than that - yes.  You may still want to consider if it is worth
forbidding "$$#@" = bla, eg. to protect the reserved variables such as the
current parser object.

[dak, 2012-08-10 16:27:34]

On 2012/08/10 16:20:30, hanwenn wrote:
> On 2012/08/10 16:16:47, dak wrote:
> > 
> > and #{ ... #} looks for "parser", and you don't want to change the name of
the
> > first argument of music functions.
> 
> the first name of the music function argument is irrelevant, as it locally
> scoped.

Wrong.  #{ ... #} expands to Lisp code taking the meaning of "parser" from the
current lexical scope rather than the current module.