[RFC] jscript: Fix shift/reduce conflict by removing redundant FunctionDeclaration rule.

[Rob Shearman]

The rule is implemented by FunctionExpression which is reduced using
the Statement rule.
---
 dlls/jscript/parser.y |   37 -------------------------------------
 1 files changed, 0 insertions(+), 37 deletions(-)

The tests pass with this change, but I haven't looked in detail as to
whether it will break anything. Jacek, can you check that your
applications that use jscript still work after this patch is applied?

[Jacek Caban]

Hi Rob,

Rob Shearman wrote:
> The rule is implemented by FunctionExpression which is reduced using
> the Statement rule.
> ---
>  dlls/jscript/parser.y |   37 -------------------------------------
>  1 files changed, 0 insertions(+), 37 deletions(-)
>
> The tests pass with this change, but I haven't looked in detail as to
> whether it will break anything. Jacek, can you check that your
> applications that use jscript still work after this patch is applied?

Thanks for looking at this. There is a big difference between function 
expressions and declarations in the specification. I don't know, how 
does it handle this shift/reduce conflict, but it doesn't matter. I've 
just tested native jscript behavior and (surprise!) it doesn't match 
specifications. It seems to handle all functions expressions that have 
an identifier as function declarations. It needs more tests and the fix 
fill be more complicated (similar to variable handling). I will work on 
it tomorrow.

Thanks,
    Jacek

[Rob Shearman]

2008/10/16 Jacek Caban <jacek(a)codeweavers.com>:
> Hi Rob,
>
> Rob Shearman wrote:
> > The rule is implemented by FunctionExpression which is reduced using
> > the Statement rule.
> > ---
> >  dlls/jscript/parser.y |   37 -------------------------------------
> >  1 files changed, 0 insertions(+), 37 deletions(-)
> >
> > The tests pass with this change, but I haven't looked in detail as to
> > whether it will break anything. Jacek, can you check that your
> > applications that use jscript still work after this patch is applied?
>
> Thanks for looking at this. There is a big difference between function
> expressions and declarations in the specification. I don't know, how does it
> handle this shift/reduce conflict, but it doesn't matter. I've just tested
> native jscript behavior and (surprise!) it doesn't match specifications. It
> seems to handle all functions expressions that have an identifier as
> function declarations. It needs more tests and the fix fill be more
> complicated (similar to variable handling). I will work on it tomorrow.

As of git latest there are two remaining problems with the jscript
grammar that cause warnings:
1. In SourceElements: Ambiguity as to whether to parse "kFUNCTION
Identifier ..." as either a FunctionDeclaration or a
FunctionExpression as part of a Statement.
2. In Statement: Ambiguity as to whether to parse "{ }" or "{
Identifier ..." as a Block or an ObjectLiteral.

I've looked at the spec and I see that it has a nice "lookahead not a
member of '{, function'" in the rule for ExpressionStatement, meaning
that the grammar cannot be implemented unambiguously by a LALR(1)
parser-generator like bison. :-(

However, it could be fixed by duplicating the Expression rules. At the
moment this is not practical as there are a lot of them, but I believe
they could be reduced down significantly by making use of bison's
%left and %right statements to allow all of the expression rules to be
combined into one expression rule. This expression rule could then be
duplicated and used for the case of not allowing the kIN keyword in
some of the expressions and again to not allow FunctionExpressions or
ObjectLiterals. Jacek, are you OK with deviating from the
specification like this?

-- 
Rob Shearman

[Jacek Caban]

Rob Shearman wrote:
> Jacek, are you OK with deviating from the specification like this?

Sure, I was thinking about something like this when I was writing the 
parser, but I decided that it's better to stick with documentation then 
to get something working. Now that we have tests and we may test it on 
real applications (Photoshop installers are good tests), we may clean it 
up IMO.

Jacek

[Jacek Caban]

Rob Shearman wrote:
> I've looked at the spec and I see that it has a nice "lookahead not a
> member of '{, function'" in the rule for ExpressionStatement, meaning
> that the grammar cannot be implemented unambiguously by a LALR(1)
> parser-generator like bison. :-(

I've found on MS blog that my guess that JScript treads function 
expressions with identifier as function declaration was right. I can't 
find the link now, it was somewhere on
http://blogs.msdn.com/jscript/
I've sent a patch that implements this behavior.


Thanks,
    Jacek