[PATCH 0/2] MR10852: vbscript/tests: Cover cross-parse name redefinition semantics.
Native VBScript treats top-level Dim as silently re-declarable across ParseScriptText calls, while Const and Class names cannot be reused once taken, and Sub/Function only conflict with an existing Class of the same name. Pin these rules with cross-parse tests covering each declaration kind paired with each other kind. Wine currently diverges on: - any decl followed by Dim (errs 1041, native accepts); - existing Class followed by Const/Sub/Function (silently allowed, native errs 1041). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10852
From: Francis De Brabandere <francisdb@gmail.com> Native VBScript treats top-level Dim as silently re-declarable across ParseScriptText calls, while Const and Class names cannot be reused once taken, and Sub/Function only conflict with an existing Class of the same name. Pin these rules with cross-parse tests covering each declaration kind paired with each other kind. Wine currently diverges on: - any decl followed by Dim (errs 1041, native accepts); - existing Class followed by Const/Sub/Function (silently allowed, native errs 1041). Marked as todo_wine pending the compile.c fix. --- dlls/vbscript/tests/vbscript.c | 139 +++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index bb25ae8017d..f17bc385cb4 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -466,9 +466,20 @@ static HRESULT WINAPI ActiveScriptSite_OnStateChange(IActiveScriptSite *iface, S return E_NOTIMPL; } +static int last_script_error; + static HRESULT WINAPI ActiveScriptSite_OnScriptError(IActiveScriptSite *iface, IActiveScriptError *pscripterror) { + EXCEPINFO ei = {0}; + HRESULT hr; CHECK_EXPECT(OnScriptError); + hr = IActiveScriptError_GetExceptionInfo(pscripterror, &ei); + if(SUCCEEDED(hr)) { + last_script_error = HRESULT_CODE(ei.scode); + SysFreeString(ei.bstrSource); + SysFreeString(ei.bstrDescription); + SysFreeString(ei.bstrHelpFile); + } return E_NOTIMPL; } @@ -2901,6 +2912,133 @@ static void test_const_at_top_level(void) ok(!ref, "ref = %ld\n", ref); } +/* Cross-parse name redefinition semantics: parse declaration `first`, + * then declaration `second` on the same script object. Expect the + * second parse to either succeed (S_OK) or report `expected_err` via + * OnScriptError. todo says the assertion should match native but + * Wine currently diverges. */ +static void run_cross_parse_redef(unsigned line, const WCHAR *first, const WCHAR *second, + int expected_err, BOOL todo) +{ + IActiveScriptParse *parser; + IActiveScript *vbscript; + HRESULT hr; + + vbscript = create_vbscript(); + hr = IActiveScript_QueryInterface(vbscript, &IID_IActiveScriptParse, (void**)&parser); + ok_(__FILE__,line)(hr == S_OK, "QI(IActiveScriptParse) hr=%08lx\n", hr); + + SET_EXPECT(GetLCID); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok_(__FILE__,line)(hr == S_OK, "SetScriptSite hr=%08lx\n", hr); + CHECK_CALLED(GetLCID); + + SET_EXPECT(OnStateChange_INITIALIZED); + hr = IActiveScriptParse_InitNew(parser); + ok_(__FILE__,line)(hr == S_OK, "InitNew hr=%08lx\n", hr); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_CONNECTED); + ok_(__FILE__,line)(hr == S_OK, "SetScriptState hr=%08lx\n", hr); + CHECK_CALLED(OnStateChange_CONNECTED); + + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + hr = IActiveScriptParse_ParseScriptText(parser, first, NULL, NULL, NULL, 0, 0, 0, NULL, NULL); + ok_(__FILE__,line)(hr == S_OK, "first parse hr=%08lx for %s\n", hr, wine_dbgstr_w(first)); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + + /* The second parse may fire OnEnter/OnLeave (runtime errors) or not + * (compile-time errors). Allow them either way and only assert on + * the error code. */ + last_script_error = 0; + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + SET_EXPECT(OnScriptError); + hr = IActiveScriptParse_ParseScriptText(parser, second, NULL, NULL, NULL, 0, 0, 0, NULL, NULL); + expect_OnEnterScript = called_OnEnterScript = 0; + expect_OnLeaveScript = called_OnLeaveScript = 0; + expect_OnScriptError = called_OnScriptError = 0; + if (expected_err) { + todo_wine_if(todo) ok_(__FILE__,line)(last_script_error == expected_err, + "second parse for %s: expected err %d, got err=%d hr=%08lx\n", + wine_dbgstr_w(second), expected_err, last_script_error, hr); + } else { + todo_wine_if(todo) ok_(__FILE__,line)(hr == S_OK && last_script_error == 0, + "second parse for %s: expected S_OK, got hr=%08lx err=%d\n", + wine_dbgstr_w(second), hr, last_script_error); + } + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hr = IActiveScript_Close(vbscript); + ok_(__FILE__,line)(hr == S_OK, "Close hr=%08lx\n", hr); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parser); + IActiveScript_Release(vbscript); +} + +#define cross_parse_ok(a, b) run_cross_parse_redef(__LINE__, a, b, 0, FALSE) +#define cross_parse_ok_todo(a, b) run_cross_parse_redef(__LINE__, a, b, 0, TRUE) +#define cross_parse_err(a, b, e) run_cross_parse_redef(__LINE__, a, b, e, FALSE) +#define cross_parse_err_todo(a, b, e) run_cross_parse_redef(__LINE__, a, b, e, TRUE) + +static void test_cross_parse_name_redef(void) +{ + /* Native rule: + * - Dim is permissive: never errors when re-declared cross-parse. + * - Sub/Func re-declare each other and Dim freely; only error when + * the name already names a Class. + * - Const errors when the name is already in use by anything. + * - Class errors when the name is already in use by anything (any + * kind, including itself). + */ + static const WCHAR dim[] = L"Dim N\n"; + static const WCHAR konst[] = L"Const N = 1\n"; + static const WCHAR sub[] = L"Sub N\nEnd Sub\n"; + static const WCHAR func[] = L"Function N\nEnd Function\n"; + static const WCHAR cls[] = L"Class N\nEnd Class\n"; + + /* Anything followed by Dim: native always accepts. Wine currently + * errors -- needs the over-aggressive var loop dropped. */ + cross_parse_ok_todo(dim, dim); + cross_parse_ok_todo(konst, dim); + cross_parse_ok_todo(sub, dim); + cross_parse_ok_todo(func, dim); + cross_parse_ok_todo(cls, dim); + + /* Class-then-X: native errors for Const/Sub/Func because the name + * is taken. Wine currently allows -- needs added cross-parse check. */ + cross_parse_err_todo(cls, konst, 1041); + cross_parse_err_todo(cls, sub, 1041); + cross_parse_err_todo(cls, func, 1041); + cross_parse_err (cls, cls, 1041); /* Wine already catches this */ + + /* Cases Wine and native already agree on. */ + cross_parse_ok (dim, sub); + cross_parse_ok (dim, func); + cross_parse_ok (sub, sub); + cross_parse_ok (sub, func); + cross_parse_ok (func, sub); + cross_parse_ok (func, func); + cross_parse_ok (konst, sub); + cross_parse_ok (konst, func); + cross_parse_err (dim, konst, 1041); + cross_parse_err (dim, cls, 1041); + cross_parse_err (konst, konst, 1041); + cross_parse_err (konst, cls, 1041); + cross_parse_err (sub, konst, 1041); + cross_parse_err (sub, cls, 1041); + cross_parse_err (func, konst, 1041); + cross_parse_err (func, cls, 1041); +} + static void test_RegExp(void) { IRegExp2 *regexp; @@ -3127,6 +3265,7 @@ START_TEST(vbscript) test_named_item_dim_shadowing(); test_named_item_no_dim_routes_to_host(); test_const_at_top_level(); + test_cross_parse_name_redef(); test_scriptdisp(); test_code_persistence(); test_script_typeinfo(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10852
From: Francis De Brabandere <francisdb@gmail.com> Drop the over-aggressive var loop in check_script_collisions so that top-level Dim re-declarations across separate ParseScriptText calls silently succeed (matching native VBScript). Add cross-parse class collision checks for new Sub/Function declarations and Const decls so they error 1041 when the name is already taken by an existing class (also matching native). Removes the todo_wine markers added by the previous commit. --- dlls/vbscript/compile.c | 59 +++++++++++++++++++++++++++++----- dlls/vbscript/tests/vbscript.c | 31 ++++++++---------- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 3c347f72686..0b3fbaf97d9 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -2213,22 +2213,65 @@ static BOOL lookup_script_identifier(compile_ctx_t *ctx, script_ctx_t *script, c return FALSE; } -static HRESULT check_script_collisions(compile_ctx_t *ctx, script_ctx_t *script) +/* Returns TRUE if `name` matches a class declared in any prior parse + * still kept on this script. Used to enforce native semantics where + * Sub/Function/Const declarations cannot reuse an existing class name. */ +static BOOL lookup_existing_class(compile_ctx_t *ctx, script_ctx_t *script, const WCHAR *name) { - unsigned i, var_cnt = ctx->code->main_code.var_cnt; - var_desc_t *vars = ctx->code->main_code.vars; + ScriptDisp *contexts[] = { + ctx->code->named_item ? ctx->code->named_item->script_obj : NULL, + script->script_obj + }; class_desc_t *class; + vbscode_t *code; + unsigned c; - for(i = 0; i < var_cnt; i++) { - if(lookup_script_identifier(ctx, script, vars[i].name)) { - return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + for(c = 0; c < ARRAY_SIZE(contexts); c++) { + if(!contexts[c]) continue; + for(class = contexts[c]->classes; class; class = class->next) { + if(!vbs_wcsicmp(class->name, name)) + return TRUE; + } + } + + LIST_FOR_EACH_ENTRY(code, &script->code_list, vbscode_t, entry) { + if(!code->pending_exec || (code->named_item && code->named_item != ctx->code->named_item)) + continue; + for(class = code->classes; class; class = class->next) { + if(!vbs_wcsicmp(class->name, name)) + return TRUE; } } + return FALSE; +} + +static HRESULT check_script_collisions(compile_ctx_t *ctx, script_ctx_t *script) +{ + class_desc_t *class; + function_t *func; + const_decl_t *konst; + + /* Native rule: top-level Dim is permissive and never errors when + * cross-parse re-declared, so we don't check vars here. Const is + * caught at runtime by interp_const, so we don't double-check. + * + * What this layer enforces: declaring a new Class, or a Sub / + * Function / Const, when the name is already used by a Class from + * a previous parse. */ for(class = ctx->code->classes; class; class = class->next) { - if(lookup_script_identifier(ctx, script, class->name)) { + if(lookup_script_identifier(ctx, script, class->name)) + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + } + + for(func = ctx->code->funcs; func; func = func->next) { + if(lookup_existing_class(ctx, script, func->name)) + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + } + + for(konst = ctx->const_decls; konst; konst = konst->next) { + if(lookup_existing_class(ctx, script, konst->name)) return MAKE_VBSERROR(VBSE_NAME_REDEFINED); - } } return S_OK; diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index f17bc385cb4..b247e9060be 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -2915,10 +2915,9 @@ static void test_const_at_top_level(void) /* Cross-parse name redefinition semantics: parse declaration `first`, * then declaration `second` on the same script object. Expect the * second parse to either succeed (S_OK) or report `expected_err` via - * OnScriptError. todo says the assertion should match native but - * Wine currently diverges. */ + * OnScriptError. */ static void run_cross_parse_redef(unsigned line, const WCHAR *first, const WCHAR *second, - int expected_err, BOOL todo) + int expected_err) { IActiveScriptParse *parser; IActiveScript *vbscript; @@ -2962,11 +2961,11 @@ static void run_cross_parse_redef(unsigned line, const WCHAR *first, const WCHAR expect_OnLeaveScript = called_OnLeaveScript = 0; expect_OnScriptError = called_OnScriptError = 0; if (expected_err) { - todo_wine_if(todo) ok_(__FILE__,line)(last_script_error == expected_err, + ok_(__FILE__,line)(last_script_error == expected_err, "second parse for %s: expected err %d, got err=%d hr=%08lx\n", wine_dbgstr_w(second), expected_err, last_script_error, hr); } else { - todo_wine_if(todo) ok_(__FILE__,line)(hr == S_OK && last_script_error == 0, + ok_(__FILE__,line)(hr == S_OK && last_script_error == 0, "second parse for %s: expected S_OK, got hr=%08lx err=%d\n", wine_dbgstr_w(second), hr, last_script_error); } @@ -2984,10 +2983,8 @@ static void run_cross_parse_redef(unsigned line, const WCHAR *first, const WCHAR IActiveScript_Release(vbscript); } -#define cross_parse_ok(a, b) run_cross_parse_redef(__LINE__, a, b, 0, FALSE) -#define cross_parse_ok_todo(a, b) run_cross_parse_redef(__LINE__, a, b, 0, TRUE) -#define cross_parse_err(a, b, e) run_cross_parse_redef(__LINE__, a, b, e, FALSE) -#define cross_parse_err_todo(a, b, e) run_cross_parse_redef(__LINE__, a, b, e, TRUE) +#define cross_parse_ok(a, b) run_cross_parse_redef(__LINE__, a, b, 0) +#define cross_parse_err(a, b, e) run_cross_parse_redef(__LINE__, a, b, e) static void test_cross_parse_name_redef(void) { @@ -3007,17 +3004,17 @@ static void test_cross_parse_name_redef(void) /* Anything followed by Dim: native always accepts. Wine currently * errors -- needs the over-aggressive var loop dropped. */ - cross_parse_ok_todo(dim, dim); - cross_parse_ok_todo(konst, dim); - cross_parse_ok_todo(sub, dim); - cross_parse_ok_todo(func, dim); - cross_parse_ok_todo(cls, dim); + cross_parse_ok(dim, dim); + cross_parse_ok(konst, dim); + cross_parse_ok(sub, dim); + cross_parse_ok(func, dim); + cross_parse_ok(cls, dim); /* Class-then-X: native errors for Const/Sub/Func because the name * is taken. Wine currently allows -- needs added cross-parse check. */ - cross_parse_err_todo(cls, konst, 1041); - cross_parse_err_todo(cls, sub, 1041); - cross_parse_err_todo(cls, func, 1041); + cross_parse_err(cls, konst, 1041); + cross_parse_err(cls, sub, 1041); + cross_parse_err(cls, func, 1041); cross_parse_err (cls, cls, 1041); /* Wine already catches this */ /* Cases Wine and native already agree on. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10852
@jacek this is a regression introduced by !10464 so more urgent than other mr's -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10852#note_139347
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10852
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)