`*` in token type

[maddyblue]

I was trying to parse cockroachdb's sql.y and hit a bug:

sql.y:454:9: unexpected '*', expected identifier

https://github.com/cockroachdb/cockroach/blob/9cf7b53242e4680af2755102da33f6c558ae501e/pkg/sql/parser/sql.y#L454

It appears this doesn't like the * in %token <*tree.NumVal> ICONST FCONST.

[cznic]

*token.Numval is not a valid goyacc identifier. I don't know what for is the * presented there. Please explain the intent/purpose of the *.

FTR, here's the lexical grammar for goyacc identifiers.

It's quite possible that yacc accepts it, but this is the first time I see something like that.

[cznic]

Also here it seems like the type tag refers to the union member name, ie. a C identifier.

Once YYSTYPE is defined, the union member names must be associated with the various terminal and nonterminal names. The construction

        < name >
is used to indicate a union member name. If this follows one of the keywords %token, %left, %right, and %nonassoc, the union
member name is associated with the tokens listed. Thus, saying

        %left  <optype>  '+'  '-'
will cause any reference to values returned by these two tokens to be tagged with the union member name optype. Another keyword, %type, is used similarly to associate union member names with nonterminals. Thus, one might say
        %type  <nodetype>  expr  stat

[maddyblue]

*tree.NumVal is the go type of those statements. It works in go tools goyacc. Just thought I'd make you aware.

[cznic]

That's pretty non-standard (cf. the link above), but worse, it's not necessary. The type is determined by the type of the union member named whatever in <whatever>.

If it would enable something that's not possible otherwise, even if non-standard, I'd possibly consider adding it to goyacc. But that does not seem to be the case, according to what you wrote. Maybe I'm mistaken?

Just to reiterate, the POSIX compatible way in this case would be

%union{
        bar *foo; // Just an example
        // ... more fields possibly
}

%token <foo> QUX // QUX has type *bar

%%

...

goyacc just reverses the type/name so it's non standard, but other than writing

%union{
        foo *bar // Just an example
        // ... more fields possibly
}

instead, the effect would be the same. Instead of *bar it can be any Go type, like eg. map[string]int etc.

Thanks for the heads up. Do you think there's something to be done about this?

[maddyblue]

This was my misunderstanding. CockroachDB (I wasn't aware of this when I made this issue) processes our sql.y into another file that is then passed to goyacc, so the above issue indeed isn't real.

[cznic]

Thanks a lot for the clarification. I'm glad it resolves the issue.