Sub / Function can be on one line, but only if there's brackets.
``` | Storage_opt tFUNCTION Identifier ArgumentsDecl BodyStatements tEND tFUNCTION ``` This leads to sometimes to `link_statements` being called with `head` being NULL, probably because now the `:` can be parsed as `Statement` in `BodyStatements`. Also note that this introduces another shift/reduce conflict, could be related, but I don't understand it well enough. Feedback would be appreciated, I'm still learning bison.
From: Fabian Maurer dark.shadow4@web.de
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54978 --- dlls/vbscript/parser.y | 14 ++++++++++++-- dlls/vbscript/tests/lang.vbs | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y index 9ae6c478bfa..89a565aec09 100644 --- a/dlls/vbscript/parser.y +++ b/dlls/vbscript/parser.y @@ -145,7 +145,7 @@ static statement_t *link_statements(statement_t*,statement_t*); %type <member> MemberExpression %type <expression> Arguments ArgumentList ArgumentList_opt Step_opt ExpressionList %type <boolean> DoType Preserve_opt -%type <arg_decl> ArgumentsDecl_opt ArgumentDeclList ArgumentDecl +%type <arg_decl> ArgumentsDecl ArgumentsDecl_opt ArgumentDeclList ArgumentDecl %type <func_decl> FunctionDecl PropertyDecl %type <elseif> ElseIfs_opt ElseIfs ElseIf %type <class_decl> ClassDeclaration ClassBody @@ -477,8 +477,12 @@ PropertyDecl FunctionDecl : Storage_opt tSUB Identifier ArgumentsDecl_opt StSep BodyStatements tEND tSUB { $$ = new_function_decl(ctx, $3, FUNC_SUB, $1, $4, $6); CHECK_ERROR; } + | Storage_opt tSUB Identifier ArgumentsDecl BodyStatements tEND tSUB + { $$ = new_function_decl(ctx, $3, FUNC_SUB, $1, $4, $5); CHECK_ERROR; } | Storage_opt tFUNCTION Identifier ArgumentsDecl_opt StSep BodyStatements tEND tFUNCTION { $$ = new_function_decl(ctx, $3, FUNC_FUNCTION, $1, $4, $6); CHECK_ERROR; } + | Storage_opt tFUNCTION Identifier ArgumentsDecl BodyStatements tEND tFUNCTION + { $$ = new_function_decl(ctx, $3, FUNC_FUNCTION, $1, $4, $5); CHECK_ERROR; }
Storage_opt : /* empty*/ { $$ = 0; } @@ -490,7 +494,11 @@ Storage | tPRIVATE { $$ = STORAGE_IS_PRIVATE; }
ArgumentsDecl_opt - : EmptyBrackets_opt { $$ = NULL; } + : /* empty*/ { $$ = 0; } + | ArgumentsDecl { $$ = $1; } + +ArgumentsDecl + : tEMPTYBRACKETS { $$ = NULL; } | '(' ArgumentDeclList ')' { $$ = $2; }
ArgumentDeclList @@ -1163,6 +1171,8 @@ static statement_t *link_statements(statement_t *head, statement_t *tail) { statement_t *iter;
+ if (!head) return tail; + for(iter = head; iter->next; iter = iter->next); iter->next = tail;
diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 77e132133b0..55efdcb5a00 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -827,6 +827,14 @@ x = false Call testsub Call ok(x, "x is false, testsub not called?")
+if false then +Sub testsub_one_line() x = true End Sub +end if + +x = false +Call testsub_one_line +Call ok(x, "x is false, testsub_one_line not called?") + Sub SubSetTrue(v) Call ok(not v, "v is not true") v = true @@ -920,6 +928,14 @@ x = false Call TestFunc Call ok(x, "x is false, testfunc not called?")
+if false then +Function testfunc_one_line() x = true End Function +end if + +x = false +Call testfunc_one_line +Call ok(x, "x is false, testfunc_one_line not called?") + Function FuncSetTrue(v) Call ok(not v, "v is not true") v = true
Also note that this introduces another shift/reduce conflict, could be related, but I don't understand it well enough. Feedback would be appreciated, I'm still learning bison.
That's because of how you use ArgumentsDecl_opt and ArgumentsDecl. I think that we could separate rules by arguments presence, something like https://gitlab.winehq.org/jacek/wine/-/commit/ab1870f29f941492b1965316859740... should avoid the problem.
On Mon Jul 3 17:13:53 2023 +0000, Jacek Caban wrote:
Also note that this introduces another shift/reduce conflict, could be
related, but I don't understand it well enough. Feedback would be appreciated, I'm still learning bison. That's because of how you use ArgumentsDecl_opt and ArgumentsDecl. I think that we could separate rules by arguments presence, something like https://gitlab.winehq.org/jacek/wine/-/commit/ab1870f29f941492b1965316859740... should avoid the problem.
Thanks, that seems sensible. Although it doesn't get rid of the nullpointer during parsing of `BodyStatements`. How we go from here?
On Mon Jul 3 17:13:53 2023 +0000, Fabian Maurer wrote:
Thanks, that seems sensible. Although it doesn't get rid of the nullpointer during parsing of `BodyStatements`. How we go from here?
Please update the MR.
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Jacek Caban (＠jacek)