[PATCH v8 0/2] MR10897: vbscript: Handle declaration scope in the parser.
ClassDeclaration was only accepted at the top SourceElement level, so a prior statement on the same line (e.g. Dim x : Class C) failed to parse instead of being accepted or, for a name collision, reported as err 1041. Make ClassDeclaration a SimpleStatement and register it as a class at compile time in source order. Sub, Function and Class declarations are only valid at script global scope. A global If/ElseIf/Else/Select block still hoists a Sub or Function to global scope, but a loop (For/For Each/While/Do) or With block does not, and a Class is never hoisted. Reject the disallowed cases during parsing, reporting the location native reports rather than failing later during bytecode emission. -- v8: vbscript: Handle declaration scope in the parser. vbscript/tests: Add tests for declaration scope handling. https://gitlab.winehq.org/wine/wine/-/merge_requests/10897
From: Francis De Brabandere <francisdb@gmail.com> A Class declaration may follow a statement separated by ':' at global scope, and a global If/Select hoists a Sub or Function. A Sub, Function or Class inside a procedure body, a Class inside any control-flow block, and a Sub or Function inside a global loop are all rejected with err 1002; a duplicate Class name is reported at the later declaration. --- dlls/vbscript/tests/run.c | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index f54efd099e2..f434daf63d7 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -3660,6 +3660,54 @@ static void test_external_caller_method_error(void) CHECK_CALLED(OnScriptError); } +static void test_class_decl_scope(void) +{ + static const struct { + const WCHAR *src; + BOOL expect_ok; /* whether the script should compile */ + USHORT error_code; /* expected error number when it should not */ + ULONG error_line; /* expected 0-based error line when it should not */ + BOOL todo; + } tests[] = { + /* A Class declaration may follow another statement separated by ':'. */ + { L"Dim x : Class C\nPublic v\nEnd Class\n", TRUE, 0, 0, TRUE }, + /* A global control-flow conditional hoists a Sub or Function. */ + { L"If False Then\nSub S\nx = 1\nEnd Sub\nEnd If\n", TRUE, 0, 0, FALSE }, + /* A Class declared inside a procedure body is rejected. */ + { L"Sub S\nClass C\nEnd Class\nEnd Sub\n", FALSE, 1002, 1, TRUE }, + /* A Sub declared inside another procedure body is rejected. */ + { L"Sub S\nSub T\nEnd Sub\nEnd Sub\n", FALSE, 1002, 1, TRUE }, + /* A Class declared inside a control-flow block is rejected. */ + { L"If True Then\nClass C\nEnd Class\nEnd If\n", FALSE, 1002, 1, TRUE }, + /* A Sub inside a global loop is not hoisted. */ + { L"For i = 1 To 1\nSub S\nEnd Sub\nNext\n", FALSE, 1002, 1, TRUE }, + /* A duplicate Class name is reported at the later declaration. */ + { L"Class C\nEnd Class\nDim x : Class C\nEnd Class\n", FALSE, 1041, 2, TRUE }, + }; + HRESULT hres; + unsigned i; + BOOL pass; + + for (i = 0; i < ARRAY_SIZE(tests); i++) { + error_line = ~0; + error_code = 0; + onerror_hres = S_OK; + SET_EXPECT(OnScriptError); + hres = parse_script_wr(tests[i].src); + CLEAR_CALLED(OnScriptError); + + if (tests[i].expect_ok) + pass = hres == S_OK; + else + pass = FAILED(hres) && error_code == tests[i].error_code + && error_line == tests[i].error_line; + + todo_wine_if(tests[i].todo) + ok(pass, "[%u] %s: hres=%08lx code=%u line=%lu\n", i, wine_dbgstr_w(tests[i].src), + hres, error_code, error_line); + } +} + static void test_msgbox(void) { HRESULT hres; @@ -4356,6 +4404,7 @@ static void run_tests(void) test_isexpression(); test_option_explicit_errors(); test_parse_errors(); + test_class_decl_scope(); test_redefine_scope(); test_getref_error_reporting(); test_getref_external_caller_error(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10897
From: Francis De Brabandere <francisdb@gmail.com> ClassDeclaration was only accepted at the top SourceElement level, so a prior statement on the same line (e.g. Dim x : Class C) failed to parse instead of being accepted or, for a name collision, reported as err 1041. Make ClassDeclaration a SimpleStatement and register it as a class at compile time in source order. Sub, Function and Class declarations are only valid at script global scope. A global If/ElseIf/Else/Select block still hoists a Sub or Function to global scope, but a loop (For/For Each/While/Do) or With block does not, and a Class is never hoisted. Reject the disallowed cases during parsing, reporting the location native reports rather than failing later during bytecode emission. --- dlls/vbscript/compile.c | 16 +++++ dlls/vbscript/parse.h | 6 ++ dlls/vbscript/parser.y | 125 ++++++++++++++++++++++++++++++++++++-- dlls/vbscript/tests/run.c | 27 ++++---- 4 files changed, 152 insertions(+), 22 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 8aef25f21b1..a23be643815 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -1441,6 +1441,19 @@ static HRESULT compile_function_statement(compile_ctx_t *ctx, function_statement return S_OK; } +static HRESULT compile_class_statement(compile_ctx_t *ctx, class_statement_t *stat) +{ + class_decl_t **iter; + + /* The parser only ever produces a class statement at global scope. Keep + source order so a duplicate name is reported at the later declaration, + matching native. */ + stat->class_decl->next = NULL; + for(iter = &ctx->parser.class_decls; *iter; iter = &(*iter)->next); + *iter = stat->class_decl; + return S_OK; +} + static HRESULT compile_exitdo_statement(compile_ctx_t *ctx) { statement_ctx_t *iter; @@ -1712,6 +1725,9 @@ static HRESULT compile_statement(compile_ctx_t *ctx, statement_ctx_t *stat_ctx, case STAT_FUNC: hres = compile_function_statement(ctx, (function_statement_t*)stat); break; + case STAT_CLASS: + hres = compile_class_statement(ctx, (class_statement_t*)stat); + break; case STAT_IF: hres = compile_if_statement(ctx, (if_statement_t*)stat); break; diff --git a/dlls/vbscript/parse.h b/dlls/vbscript/parse.h index 4febe0ca535..66451c1cec0 100644 --- a/dlls/vbscript/parse.h +++ b/dlls/vbscript/parse.h @@ -112,6 +112,7 @@ typedef struct { typedef enum { STAT_ASSIGN, STAT_CALL, + STAT_CLASS, STAT_CONST, STAT_DIM, STAT_DOUNTIL, @@ -223,6 +224,11 @@ typedef struct _class_decl_t { struct _class_decl_t *next; } class_decl_t; +typedef struct { + statement_t stat; + class_decl_t *class_decl; +} class_statement_t; + typedef struct _elseif_decl_t { expression_t *expr; statement_t *stat; diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y index ec67cde2215..3d3897c7510 100644 --- a/dlls/vbscript/parser.y +++ b/dlls/vbscript/parser.y @@ -30,7 +30,6 @@ static int parser_error(unsigned*,parser_ctx_t*,const char*); static void handle_isexpression_script(parser_ctx_t *ctx, expression_t *expr); static void source_add_statement(parser_ctx_t*,statement_t*); -static void source_add_class(parser_ctx_t*,class_decl_t*); static void *new_expression(parser_ctx_t*,expression_type_t,size_t); static expression_t *new_bool_expression(parser_ctx_t*,VARIANT_BOOL); @@ -58,6 +57,7 @@ static statement_t *new_forto_statement(parser_ctx_t*,unsigned,const WCHAR*,expr static statement_t *new_foreach_statement(parser_ctx_t*,unsigned,const WCHAR*,expression_t*,statement_t*); static statement_t *new_if_statement(parser_ctx_t*,unsigned,expression_t*,statement_t*,elseif_decl_t*,statement_t*); static statement_t *new_function_statement(parser_ctx_t*,unsigned,function_decl_t*); +static statement_t *new_class_statement(parser_ctx_t*,unsigned,class_decl_t*); static statement_t *new_onerror_statement(parser_ctx_t*,unsigned,BOOL); static statement_t *new_const_statement(parser_ctx_t*,unsigned,const_decl_t*); static statement_t *new_select_statement(parser_ctx_t*,unsigned,expression_t*,case_clausule_t*); @@ -186,7 +186,6 @@ SourceElements | SourceElements GlobalDimDeclaration StSep { source_add_statement(ctx, $2); } | SourceElements StatementNl { source_add_statement(ctx, $2); } - | SourceElements ClassDeclaration { source_add_class(ctx, $2); } GlobalDimDeclaration : tPRIVATE tCONST ConstDeclList { $$ = new_const_statement(ctx, @$, $3); CHECK_ERROR; } @@ -267,6 +266,7 @@ SimpleStatement | tDO StSep StatementsNl_opt error { ctx->hres = MAKE_VBSERROR(VBSE_EXPECTED_LOOP); YYABORT; } | tDO error { ctx->hres = MAKE_VBSERROR(VBSE_EXPECTED_WHILE_UNTIL_EOS); YYABORT; } | FunctionDecl { $$ = new_function_statement(ctx, @$, $1); CHECK_ERROR; } + | ClassDeclaration { $$ = new_class_statement(ctx, @$, $1); CHECK_ERROR; } | tEXIT tDO { $$ = new_statement(ctx, STAT_EXITDO, 0, @2); CHECK_ERROR; } | tEXIT tFOR { $$ = new_statement(ctx, STAT_EXITFOR, 0, @2); CHECK_ERROR; } | tEXIT tFUNCTION { $$ = new_statement(ctx, STAT_EXITFUNC, 0, @2); CHECK_ERROR; } @@ -574,7 +574,7 @@ PrimaryExpression | tME { $$ = new_expression(ctx, EXPR_ME, 0); CHECK_ERROR; } ClassDeclaration - : tCLASS Identifier StSep ClassBody tEND tCLASS StSep { $4->name = $2; $4->loc = @2; $$ = $4; } + : tCLASS Identifier StSep ClassBody tEND tCLASS { $4->name = $2; $4->loc = @2; $$ = $4; } | tCLASS Identifier tEND tCLASS { ctx->error_loc = @3; ctx->hres = MAKE_VBSERROR(VBSE_EXPECTED_STATEMENT); YYABORT; } | tCLASS Identifier StSep ClassBody tEND error { ctx->hres = MAKE_VBSERROR(VBSE_EXPECTED_CLASS); YYABORT; } @@ -731,10 +731,87 @@ static void source_add_statement(parser_ctx_t *ctx, statement_t *stat) } } -static void source_add_class(parser_ctx_t *ctx, class_decl_t *class_decl) +/* Sub, Function and Class declarations are only allowed at script global + scope. They may still appear inside a global control-flow block (the engine + hoists them), but not inside a procedure body. This rejects one found within + a procedure body - including nested control-flow - at parse time, reporting + the location native reports. */ +static BOOL reject_nested_decl(parser_ctx_t *ctx, statement_t *body) { - class_decl->next = ctx->class_decls; - ctx->class_decls = class_decl; + statement_t *iter; + + for(iter = body; iter; iter = iter->next) { + switch(iter->type) { + case STAT_FUNC: + /* Report at the Sub/Function keyword, not the optional storage prefix. */ + ctx->error_loc = ((function_statement_t*)iter)->func_decl->loc; + ctx->hres = MAKE_VBSERROR(VBSE_SYNTAX_ERROR); + return TRUE; + case STAT_CLASS: + ctx->error_loc = iter->loc; + ctx->hres = MAKE_VBSERROR(VBSE_SYNTAX_ERROR); + return TRUE; + case STAT_IF: { + if_statement_t *if_stat = (if_statement_t*)iter; + elseif_decl_t *elseif; + if(reject_nested_decl(ctx, if_stat->if_stat) || reject_nested_decl(ctx, if_stat->else_stat)) + return TRUE; + for(elseif = if_stat->elseifs; elseif; elseif = elseif->next) + if(reject_nested_decl(ctx, elseif->stat)) + return TRUE; + break; + } + case STAT_WHILE: + case STAT_WHILELOOP: + case STAT_DOWHILE: + case STAT_DOUNTIL: + case STAT_UNTIL: + if(reject_nested_decl(ctx, ((while_statement_t*)iter)->body)) + return TRUE; + break; + case STAT_FORTO: + if(reject_nested_decl(ctx, ((forto_statement_t*)iter)->body)) + return TRUE; + break; + case STAT_FOREACH: + if(reject_nested_decl(ctx, ((foreach_statement_t*)iter)->body)) + return TRUE; + break; + case STAT_WITH: + if(reject_nested_decl(ctx, ((with_statement_t*)iter)->body)) + return TRUE; + break; + case STAT_SELECT: { + case_clausule_t *clause; + for(clause = ((select_statement_t*)iter)->case_clausules; clause; clause = clause->next) + if(reject_nested_decl(ctx, clause->stat)) + return TRUE; + break; + } + default: + break; + } + } + + return FALSE; +} + +/* Unlike Sub/Function (which a global control-flow block hoists to global + scope), a Class declaration is never allowed inside a control-flow block. + Reject one found directly in such a body. */ +static BOOL reject_nested_class(parser_ctx_t *ctx, statement_t *body) +{ + statement_t *iter; + + for(iter = body; iter; iter = iter->next) { + if(iter->type == STAT_CLASS) { + ctx->error_loc = iter->loc; + ctx->hres = MAKE_VBSERROR(VBSE_SYNTAX_ERROR); + return TRUE; + } + } + + return FALSE; } static void handle_isexpression_script(parser_ctx_t *ctx, expression_t *expr) @@ -1080,6 +1157,9 @@ static elseif_decl_t *new_elseif_decl(parser_ctx_t *ctx, unsigned loc, expressio { elseif_decl_t *decl; + if(reject_nested_class(ctx, stat)) + return NULL; + decl = parser_alloc(ctx, sizeof(*decl)); if(!decl) return NULL; @@ -1095,6 +1175,9 @@ static statement_t *new_while_statement(parser_ctx_t *ctx, unsigned loc, stateme { while_statement_t *stat; + if(reject_nested_decl(ctx, body)) + return NULL; + stat = new_statement(ctx, type, sizeof(*stat), loc); if(!stat) return NULL; @@ -1109,6 +1192,9 @@ static statement_t *new_forto_statement(parser_ctx_t *ctx, unsigned loc, const W { forto_statement_t *stat; + if(reject_nested_decl(ctx, body)) + return NULL; + stat = new_statement(ctx, STAT_FORTO, sizeof(*stat), loc); if(!stat) return NULL; @@ -1126,6 +1212,9 @@ static statement_t *new_foreach_statement(parser_ctx_t *ctx, unsigned loc, const { foreach_statement_t *stat; + if(reject_nested_decl(ctx, body)) + return NULL; + stat = new_statement(ctx, STAT_FOREACH, sizeof(*stat), loc); if(!stat) return NULL; @@ -1141,6 +1230,9 @@ static statement_t *new_if_statement(parser_ctx_t *ctx, unsigned loc, expression { if_statement_t *stat; + if(reject_nested_class(ctx, if_stat) || reject_nested_class(ctx, else_stat)) + return NULL; + stat = new_statement(ctx, STAT_IF, sizeof(*stat), loc); if(!stat) return NULL; @@ -1169,6 +1261,9 @@ static statement_t *new_with_statement(parser_ctx_t *ctx, unsigned loc, expressi { with_statement_t *stat; + if(reject_nested_decl(ctx, body)) + return NULL; + stat = new_statement(ctx, STAT_WITH, sizeof(*stat), loc); if(!stat) return NULL; @@ -1182,6 +1277,9 @@ static case_clausule_t *new_case_clausule(parser_ctx_t *ctx, expression_t *expr, { case_clausule_t *ret; + if(reject_nested_class(ctx, stat)) + return NULL; + ret = parser_alloc(ctx, sizeof(*ret)); if(!ret) return NULL; @@ -1224,6 +1322,9 @@ static function_decl_t *new_function_decl(parser_ctx_t *ctx, unsigned loc, unsig function_decl_t *decl; BOOL is_default = FALSE; + if(reject_nested_decl(ctx, body)) + return NULL; + if(storage_flags & STORAGE_IS_DEFAULT) { if(type == FUNC_PROPGET || type == FUNC_FUNCTION || type == FUNC_SUB) { is_default = TRUE; @@ -1262,6 +1363,18 @@ static statement_t *new_function_statement(parser_ctx_t *ctx, unsigned loc, func return &stat->stat; } +static statement_t *new_class_statement(parser_ctx_t *ctx, unsigned loc, class_decl_t *decl) +{ + class_statement_t *stat; + + stat = new_statement(ctx, STAT_CLASS, sizeof(*stat), loc); + if(!stat) + return NULL; + + stat->class_decl = decl; + return &stat->stat; +} + static class_decl_t *new_class_decl(parser_ctx_t *ctx) { class_decl_t *class_decl; diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index f434daf63d7..3e147e29f0d 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -3667,26 +3667,24 @@ static void test_class_decl_scope(void) BOOL expect_ok; /* whether the script should compile */ USHORT error_code; /* expected error number when it should not */ ULONG error_line; /* expected 0-based error line when it should not */ - BOOL todo; } tests[] = { /* A Class declaration may follow another statement separated by ':'. */ - { L"Dim x : Class C\nPublic v\nEnd Class\n", TRUE, 0, 0, TRUE }, + { L"Dim x : Class C\nPublic v\nEnd Class\n", TRUE }, /* A global control-flow conditional hoists a Sub or Function. */ - { L"If False Then\nSub S\nx = 1\nEnd Sub\nEnd If\n", TRUE, 0, 0, FALSE }, + { L"If False Then\nSub S\nx = 1\nEnd Sub\nEnd If\n", TRUE }, /* A Class declared inside a procedure body is rejected. */ - { L"Sub S\nClass C\nEnd Class\nEnd Sub\n", FALSE, 1002, 1, TRUE }, + { L"Sub S\nClass C\nEnd Class\nEnd Sub\n", FALSE, 1002, 1 }, /* A Sub declared inside another procedure body is rejected. */ - { L"Sub S\nSub T\nEnd Sub\nEnd Sub\n", FALSE, 1002, 1, TRUE }, + { L"Sub S\nSub T\nEnd Sub\nEnd Sub\n", FALSE, 1002, 1 }, /* A Class declared inside a control-flow block is rejected. */ - { L"If True Then\nClass C\nEnd Class\nEnd If\n", FALSE, 1002, 1, TRUE }, + { L"If True Then\nClass C\nEnd Class\nEnd If\n", FALSE, 1002, 1 }, /* A Sub inside a global loop is not hoisted. */ - { L"For i = 1 To 1\nSub S\nEnd Sub\nNext\n", FALSE, 1002, 1, TRUE }, + { L"For i = 1 To 1\nSub S\nEnd Sub\nNext\n", FALSE, 1002, 1 }, /* A duplicate Class name is reported at the later declaration. */ - { L"Class C\nEnd Class\nDim x : Class C\nEnd Class\n", FALSE, 1041, 2, TRUE }, + { L"Class C\nEnd Class\nDim x : Class C\nEnd Class\n", FALSE, 1041, 2 }, }; HRESULT hres; unsigned i; - BOOL pass; for (i = 0; i < ARRAY_SIZE(tests); i++) { error_line = ~0; @@ -3697,14 +3695,11 @@ static void test_class_decl_scope(void) CLEAR_CALLED(OnScriptError); if (tests[i].expect_ok) - pass = hres == S_OK; + ok(hres == S_OK, "[%u] %s: hres=%08lx\n", i, wine_dbgstr_w(tests[i].src), hres); else - pass = FAILED(hres) && error_code == tests[i].error_code - && error_line == tests[i].error_line; - - todo_wine_if(tests[i].todo) - ok(pass, "[%u] %s: hres=%08lx code=%u line=%lu\n", i, wine_dbgstr_w(tests[i].src), - hres, error_code, error_line); + ok(FAILED(hres) && error_code == tests[i].error_code && error_line == tests[i].error_line, + "[%u] %s: hres=%08lx code=%u line=%lu\n", i, wine_dbgstr_w(tests[i].src), + hres, error_code, error_line); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10897
On Fri May 29 13:09:41 2026 +0000, Francis De Brabandere wrote:
Another attempt @jacek what do you think about the current solution?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10897#note_141667
On Fri May 29 13:09:41 2026 +0000, Francis De Brabandere wrote:
@jacek what do you think about the current solution? It's still adding checks that could be handled by grammar rules instead. For example, a simple change like:
- | SourceElements StatementNl { source_add_statement(ctx, $2); }
+ | SourceElements Statement StSep { source_add_statement(ctx, $2); }
is enough to allow parsing the problematic script. A precise error reporting could be probably done by changing other rules too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10897#note_141730
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)