code review 4256044: goyacc: provide -p flag to set prefix for names.

goyacc: provide -p flag to set prefix for names.

This should allow multiple goyacc grammars to be
compiled into the same binary. There's a chance it
Fixes issue 1562.

[r, 2011-03-01 00:52:18]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-03-01 01:03:56]

> (Some local hidden symbols may still begin "yy".)

codereview won't let me log in.
the code looks good except for the final sentence
in doc.go.  if you run 6nm units | grep -i yy and it
turns anything up, -p should rename that.
if not, the last sentence in doc.go can go.

russ

[r2, 2011-03-01 01:18:34]

On Feb 28, 2011, at 5:03 PM, Russ Cox wrote:

>> (Some local hidden symbols may still begin "yy".)
> 
> codereview won't let me log in.
> the code looks good except for the final sentence
> in doc.go.  if you run 6nm units | grep -i yy and it
> turns anything up, -p should rename that.
> if not, the last sentence in doc.go can go.
> 
> russ

the important word is 'local'.  i could rename all local variables if you like.

-rob

[rsc, 2011-03-01 01:19:08]

> the important word is 'local'.  i could rename all local variables if you
like.

if it's just local variables i'm not worried.
but then it should be inconsequential to the user
and can be dropped from the docs.

[r2, 2011-03-01 01:20:16]

On Feb 28, 2011, at 5:19 PM, Russ Cox wrote:

>> the important word is 'local'.  i could rename all local variables if you
like.
> 
> if it's just local variables i'm not worried.
> but then it should be inconsequential to the user
> and can be dropped from the docs.

the problem is that the rules go into the local function, so there's a remote
possibility of collision.

hell, i'll just rename them all.

-rob

[r2, 2011-03-01 01:28:12]

tubenose=% 6nm units | grep -i yy
   4fb88 D go.string."yys"
   442f8 D string."yys"
tubenose=% 

not sure what that is, unless it's reflection data. the only yys is a field of a
struct.

-rob

[r, 2011-03-01 01:35:24]

Hello rsc, r2 (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-03-01 01:39:41]

On Mon, Feb 28, 2011 at 20:28, Rob 'Commander' Pike <r@google.com> wrote:
> tubenose=% 6nm units | grep -i yy
>   4fb88 D go.string."yys"
>   442f8 D string."yys"
> tubenose=%
>
> not sure what that is, unless it's reflection data. the only yys is a field of
a struct.

that's what it is.

[rsc, 2011-03-01 01:40:12]

LGTM

[r, 2011-03-01 01:43:27]

*** Submitted as http://code.google.com/p/go/source/detail?r=f7eb42f2a25c ***

goyacc: provide -p flag to set prefix for names.

This should allow multiple goyacc grammars to be
compiled into the same binary. There's a chance it
Fixes issue 1562.

R=rsc, r2
CC=golang-dev
http://codereview.appspot.com/4256044

[r2, 2011-03-01 01:45:15]

hold on

why did i just submit this? why not just use generate each grammar into a
separate package.


-rob

[rsc, 2011-03-01 02:22:50]

> why did i just submit this? why not just use generate each grammar into a
separate package.

i think it's a good thing to avoid any requirements that
you can only have one foo per package.  for example,
we allow multiple types per package (in contrast to java).

i have a separate change i'd like to see to goyacc,
but it is more involved: goyacc should have a

%receiver x *T  (or x *t, doesn't have to be exported)

line that makes Parse() a method on (x *T) that calls
x.Lex and x.Error instead of taking an interface value.
then the prefix for generated symbols might well be yy_T_,
which would avoid the need for -p, because multiple
parsers would just have different receivers.
but it's a bigger change and this is a good fix to
what's there while we mull over whether %receiver
is a good idea.

the main benefit of the %receiver change would be
that the actions could refer to and store data in x.
i wrote a parser using goyacc a few months ago that
i wanted to be usable simultaneously from independent
goroutines, and i found there was no obvious way for
Parse to return the final value, the output of the parse.
i solved my problem by calling panic(value) and wrapping
Parse in a recover that retrieves the parsed value.
that's the only way i know to avoid storing it in a global.
%receiver would solve the same problem in a less
disgusting manner.

russ

[r2, 2011-03-01 04:51:44]

that's a nice idea

the thing i did today mimics the flags in bison, so i think it's ok to have in
any case

at least i learned today that someone else uses yacc

-rob

[rog, 2011-03-01 08:51:03]

On 1 March 2011 02:22, Russ Cox <rsc@golang.org> wrote:
>> why did i just submit this? why not just use generate each grammar into a
separate package.
>
> i think it's a good thing to avoid any requirements that
> you can only have one foo per package.  for example,
> we allow multiple types per package (in contrast to java).
>
> i have a separate change i'd like to see to goyacc,
> but it is more involved: goyacc should have a
>
> %receiver x *T  (or x *t, doesn't have to be exported)
>
> line that makes Parse() a method on (x *T) that calls
> x.Lex and x.Error instead of taking an interface value.

the down side of this is that it would become impossible
to use a lexer type declared in a separate package,
although this probably isn't a problem as you can
always embed.

there's no particular reason to conflate
the lexer and the parser. %receiver x *T
could create this parse method

func (x *T) Parse(yylex yyLexer) int

having the lexer separate means that it's easier
to use the same parser with different lexer types,
but there's no reason x and yylex might not actually be
the same object.

i wouldn't object either way.

> the main benefit of the %receiver change would be
> that the actions could refer to and store data in x.

you can use yylex for that. for instance, a reentrant
parser i wrote ages ago does this to return the
final value of the parse (in fact it was the above example
that motivated me to make go yacc reentrant in the first place.)

here, for example, is the top level production:

unit: line ';' {
		yylex.(*abcLex).result = $line;
		return 0
	}
	| line tEND {
		yylex.(*abcLex).result = $line;
		return 0
	}

no need for panic abuse, although you might not like
this either, and you're right that it's not obvious.