[PATCH v5 0/2] MR10897: Draft: vbscript: Allow Class declarations inline after Dim/Sub separated by ':'.
ClassDeclaration was only allowed at the top SourceElement level, so any prior statement on the same line caused err 1002 instead of either parsing successfully or, for a name collision, err 1041. Treat ClassDeclaration as a SimpleStatement matching the FunctionDecl pattern: it produces a STAT_CLASS statement that registers the class with the parser at compile time and errors out if the declaration is not at script global scope. -- v5: 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 decde599d0c..9ff1dffa164 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -3540,6 +3540,54 @@ static void test_redefine_scope(void) } } +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; @@ -4236,6 +4284,7 @@ static void run_tests(void) test_isexpression(); test_option_explicit_errors(); test_parse_errors(); + test_class_decl_scope(); test_redefine_scope(); test_parse_context(); test_callbacks(); -- 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 9ff1dffa164..25f1b967506 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -3547,26 +3547,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; @@ -3577,14 +3575,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 22 17:37:13 2026 +0000, Jacek Caban wrote:
I'm not sure about moving that logic to the compiler. Technically, this is the kind of thing the parser should handle. The statement separation syntax in vbscript is very nuanced, so I can see why it's tempting to move it into the compiler (that is also one of the reasons why I considered rewriting the parser in pure C at some point). I could be fine with that approach in some cases, but the additional state tracking needed to reconstruct parser state does not seem great. Note that the distinction between the parser and bytecode emitter stages is not purely internal. It has some externally visible effects. For example, with this MR, something like the following: ``` sub s() class c end class end sub ; parse error ; ``` would report a parser error in line 5 instead of failing earlier in line 2 first. (Admittedly, it's not a very important difference.) Another attempt
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10897#note_141066
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)