From: Francis De Brabandere <francisdb@gmail.com> Identifier lookups that resolve into a named item's host dispatch called GetIDsOfNames on the host for every access, while native vbscript resolves a name once and reuses the DISPID afterwards. Building on the unified member tree, collapse the lookup_global_vars and lookup_global_funcs pair into a single member lookup that also understands host properties, and record a resolved host DISPID as a SCRIPTDISP_HOSTPROP entry in the named item's script dispatch. A subsequent lookup finds the entry directly and never queries the host dispatch again. The entry lives and dies with the named item's dispatch, and is kept out of the script dispatch's own member enumeration. A later Dim or declaration of the same name shadows the cached host property, matching native precedence. This replaces the separate per-named-item DISPID cache with a single representation in the global tree. --- dlls/vbscript/interp.c | 63 +++++++++++++++--------- dlls/vbscript/tests/vbscript.c | 90 ++++++++++++++++++++++++++++++++++ dlls/vbscript/vbdisp.c | 40 +++++++++++++-- dlls/vbscript/vbscript.c | 8 ++- dlls/vbscript/vbscript.h | 9 +++- 5 files changed, 180 insertions(+), 30 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 6d107eb2419..abb340fc334 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -97,34 +97,40 @@ static BOOL lookup_dynamic_vars(dynamic_var_t *var, const WCHAR *name, ref_t *re return FALSE; } -static BOOL lookup_global_vars(ScriptDisp *script, const WCHAR *name, ref_t *ref) +static void ref_from_entry(scriptdisp_entry_t *member, ref_t *ref) { - dynamic_var_t *var = script_disp_find_var(script, name); - - if(!var) - return FALSE; - - ref->type = var->is_const ? REF_CONST : REF_VAR; - ref->u.v = &var->v; - return TRUE; + switch(member->type) { + case SCRIPTDISP_VAR: + ref->type = member->u.var->is_const ? REF_CONST : REF_VAR; + ref->u.v = &member->u.var->v; + break; + case SCRIPTDISP_FUNC: + ref->type = REF_FUNC; + ref->u.f = member->u.func; + break; + case SCRIPTDISP_HOSTPROP: + ref->type = REF_DISP; + ref->u.d.disp = member->u.host.disp; + ref->u.d.id = member->u.host.id; + break; + } } -static BOOL lookup_global_funcs(ScriptDisp *script, const WCHAR *name, ref_t *ref) +/* A global name maps to a single member, so one lookup covers variables, + functions and host properties cached through a named item's dispatch. */ +static scriptdisp_entry_t *lookup_global_member(ScriptDisp *script, const WCHAR *name, ref_t *ref) { - function_t *func = script_disp_find_func(script, name); + scriptdisp_entry_t *member = script_disp_find_member(script, name); - if(func) { - ref->type = REF_FUNC; - ref->u.f = func; - return TRUE; - } - - return FALSE; + if(member) + ref_from_entry(member, ref); + return member; } static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_t invoke_type, ref_t *ref) { ScriptDisp *script_obj = ctx->script->script_obj; + scriptdisp_entry_t *member; named_item_t *item; unsigned i; DISPID id; @@ -202,9 +208,7 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ } if(ctx->code->named_item) { - if(lookup_global_vars(ctx->code->named_item->script_obj, name, ref)) - return S_OK; - if(lookup_global_funcs(ctx->code->named_item->script_obj, name, ref)) + if(lookup_global_member(ctx->code->named_item->script_obj, name, ref)) return S_OK; } @@ -213,6 +217,10 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ { hres = disp_get_id(ctx->func->code_ctx->named_item->disp, name, invoke_type, TRUE, &id); if(SUCCEEDED(hres)) { + /* Cache the host property in the item's tree; subsequent lookups + find it directly and never query the host dispatch again. */ + script_disp_add_hostprop(ctx->code->named_item->script_obj, name, + ctx->func->code_ctx->named_item->disp, id); ref->type = REF_DISP; ref->u.d.disp = ctx->func->code_ctx->named_item->disp; ref->u.d.id = id; @@ -220,9 +228,7 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ } } - if(lookup_global_vars(script_obj, name, ref)) - return S_OK; - if(lookup_global_funcs(script_obj, name, ref)) + if(lookup_global_member(script_obj, name, ref)) return S_OK; hres = get_builtin_id(ctx->script->global_obj, name, &id); @@ -242,8 +248,17 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ LIST_FOR_EACH_ENTRY(item, &ctx->script->named_items, named_item_t, entry) { if((item->flags & SCRIPTITEM_GLOBALMEMBERS)) { + if(item->script_obj && (member = script_disp_find_member(item->script_obj, name)) + && member->type == SCRIPTDISP_HOSTPROP) { + ref->type = REF_DISP; + ref->u.d.disp = member->u.host.disp; + ref->u.d.id = member->u.host.id; + return S_OK; + } hres = disp_get_id(item->disp, name, invoke_type, FALSE, &id); if(SUCCEEDED(hres)) { + if(item->script_obj) + script_disp_add_hostprop(item->script_obj, name, item->disp, id); ref->type = REF_DISP; ref->u.d.disp = item->disp; ref->u.d.id = id; diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 6fe7893d3b0..9162435c7b8 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -304,15 +304,19 @@ static const IDispatchVtbl persistent_named_item_vtbl = { static IDispatch persistent_named_item = { &persistent_named_item_vtbl }; +static int shadow_method_dispid_queries, shadow_prop_dispid_queries; + static HRESULT WINAPI shadowing_GetIDsOfNames(IDispatch *iface, REFIID riid, LPOLESTR *names, UINT name_cnt, LCID lcid, DISPID *ids) { ok(name_cnt == 1, "name_cnt = %u\n", name_cnt); if(!wcscmp(names[0], L"shadowMethod")) { + shadow_method_dispid_queries++; *ids = 1; return S_OK; } if(!wcscmp(names[0], L"shadowProp")) { + shadow_prop_dispid_queries++; *ids = 2; return S_OK; } @@ -3080,6 +3084,91 @@ static void test_named_item_no_dim_routes_to_host(void) ok(!ref, "ref = %ld\n", ref); } +static void test_named_item_dispid_caching(void) +{ + IActiveScriptParse *parse; + IActiveScript *script; + HRESULT hres; + LONG ref; + + /* The engine resolves a host member name through GetIDsOfNames once and + * reuses the DISPID for subsequent lookups, within and across script + * chunks. */ + + script = create_vbscript(); + hres = IActiveScript_QueryInterface(script, &IID_IActiveScriptParse, (void**)&parse); + ok(hres == S_OK, "Could not get IActiveScriptParse: %08lx\n", hres); + + SET_EXPECT(GetLCID); + hres = IActiveScript_SetScriptSite(script, &ActiveScriptSite); + ok(hres == S_OK, "SetScriptSite failed: %08lx\n", hres); + CHECK_CALLED(GetLCID); + + hres = IActiveScript_AddNamedItem(script, L"shadowingItem", SCRIPTITEM_ISVISIBLE); + ok(hres == S_OK, "AddNamedItem failed: %08lx\n", hres); + + SET_EXPECT(OnStateChange_INITIALIZED); + hres = IActiveScriptParse_InitNew(parse); + ok(hres == S_OK, "InitNew failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hres = IActiveScript_SetScriptState(script, SCRIPTSTATE_CONNECTED); + ok(hres == S_OK, "SetScriptState failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_CONNECTED); + + shadow_method_dispid_queries = 0; + shadow_prop_dispid_queries = 0; + + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + SET_EXPECT(host_shadow_propput); + SET_EXPECT(host_shadow_propget); + SET_EXPECT(host_shadow_method); + hres = IActiveScriptParse_ParseScriptText(parse, L"shadowProp = 5\nx = shadowProp\nshadowMethod\n", + L"shadowingItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + CHECK_CALLED(host_shadow_propput); + CHECK_CALLED(host_shadow_propget); + CHECK_CALLED(host_shadow_method); + + ok(shadow_prop_dispid_queries == 1, "shadowProp resolved %d times, expected 1\n", shadow_prop_dispid_queries); + ok(shadow_method_dispid_queries == 1, "shadowMethod resolved %d times, expected 1\n", shadow_method_dispid_queries); + + /* a later script chunk reuses the cached ids too */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + SET_EXPECT(host_shadow_propput); + SET_EXPECT(host_shadow_method); + hres = IActiveScriptParse_ParseScriptText(parse, L"shadowProp = 6\nshadowMethod\n", + L"shadowingItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + CHECK_CALLED(host_shadow_propput); + CHECK_CALLED(host_shadow_method); + + ok(shadow_prop_dispid_queries == 1, "shadowProp resolved %d times after second chunk, expected 1\n", + shadow_prop_dispid_queries); + ok(shadow_method_dispid_queries == 1, "shadowMethod resolved %d times after second chunk, expected 1\n", + shadow_method_dispid_queries); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + hres = IActiveScript_Close(script); + ok(hres == S_OK, "Close failed: %08lx\n", hres); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); + + IActiveScriptParse_Release(parse); + ref = IActiveScript_Release(script); + ok(!ref, "ref = %ld\n", ref); +} + static void test_const_at_top_level(void) { IActiveScriptParse *parse; @@ -3521,6 +3610,7 @@ START_TEST(vbscript) test_named_item_sub_shadowing(); test_named_item_dim_shadowing(); test_named_item_no_dim_routes_to_host(); + test_named_item_dispid_caching(); test_const_at_top_level(); test_cross_parse_name_redef(); test_scriptdisp(); diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c index 790dad2604d..e99e24e831d 100644 --- a/dlls/vbscript/vbdisp.c +++ b/dlls/vbscript/vbdisp.c @@ -82,6 +82,36 @@ scriptdisp_entry_t *script_disp_add_func(ScriptDisp *disp, function_t *func) return member; } +/* Cache a host property resolved through a named item's dispatch. Returns NULL + without caching when a real variable or function already owns the name, so a + later script declaration is never shadowed by a stale host entry. */ +scriptdisp_entry_t *script_disp_add_hostprop(ScriptDisp *disp, const WCHAR *name, IDispatch *disp_obj, DISPID id) +{ + scriptdisp_entry_t *member = script_disp_find_member(disp, name); + + if (member) { + if (member->type != SCRIPTDISP_HOSTPROP) + return NULL; + } else { + WCHAR *str; + size_t size = (lstrlenW(name) + 1) * sizeof(WCHAR); + + /* The name must outlive the entry; the caller's bytecode string may not, + so keep a copy in the dispatch's own pool. */ + if (!(str = heap_pool_alloc(&disp->heap, size))) + return NULL; + memcpy(str, name, size); + + if (!(member = script_disp_add_member(disp, str))) + return NULL; + } + + member->type = SCRIPTDISP_HOSTPROP; + member->u.host.disp = disp_obj; + member->u.host.id = id; + return member; +} + function_t *script_disp_find_func(ScriptDisp *disp, const WCHAR *name) { scriptdisp_entry_t *member = script_disp_find_member(disp, name); @@ -1785,11 +1815,15 @@ static HRESULT WINAPI ScriptDisp_GetDispID(IDispatchEx *iface, BSTR bstrName, DW member = script_disp_find_member(This, bstrName); if(member) { - if(member->type == SCRIPTDISP_FUNC) + /* Host properties cached on this dispatch are not script members. */ + if(member->type == SCRIPTDISP_FUNC) { *pid = member->u.func->index + 1 + DISPID_FUNCTION_MASK; - else + return S_OK; + } + if(member->type == SCRIPTDISP_VAR) { *pid = member->u.var->index + 1; - return S_OK; + return S_OK; + } } *pid = -1; diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index 66deef0de6c..a4cfeea9eab 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -145,8 +145,12 @@ HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL /* Dim is permissive: it keeps an existing variable or function of the same name. A const from a previous compile unit is the exception: it is shadowed by a fresh variable that name lookups resolve to from now - on, while the defining compile unit keeps using the inlined value. */ - if (existing && (existing->type != SCRIPTDISP_VAR || !existing->u.var->is_const)) + on, while the defining compile unit keeps using the inlined value. A + cached host property is likewise shadowed by the fresh variable, which + outranks the host dispatch in the named item scope. */ + if (existing && existing->type == SCRIPTDISP_FUNC) + continue; + if (existing && existing->type == SCRIPTDISP_VAR && !existing->u.var->is_const) continue; if (!(var = heap_pool_alloc(&obj->heap, sizeof(*var)))) diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 287ff62c1f1..1626940e869 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -125,11 +125,13 @@ typedef struct _dynamic_var_t { typedef enum { SCRIPTDISP_VAR, SCRIPTDISP_FUNC, + SCRIPTDISP_HOSTPROP, } scriptdisp_entry_type_t; /* A single named member of a ScriptDisp. A global name resolves to exactly one member, so variables and functions share one tree; the payload selected - by type can grow to cover further kinds (e.g. cached host properties). */ + by type also caches a host property resolved through a named item's + dispatch, so a name found on the host is looked up only once. */ typedef struct { struct rb_entry entry; const WCHAR *name; @@ -137,6 +139,10 @@ typedef struct { union { dynamic_var_t *var; function_t *func; + struct { + IDispatch *disp; + DISPID id; + } host; } u; } scriptdisp_entry_t; @@ -166,6 +172,7 @@ typedef struct { scriptdisp_entry_t *script_disp_find_member(ScriptDisp *disp, const WCHAR *name); scriptdisp_entry_t *script_disp_add_var(ScriptDisp *disp, dynamic_var_t *var); scriptdisp_entry_t *script_disp_add_func(ScriptDisp *disp, function_t *func); +scriptdisp_entry_t *script_disp_add_hostprop(ScriptDisp *disp, const WCHAR *name, IDispatch *disp_obj, DISPID id); dynamic_var_t *script_disp_find_var(ScriptDisp *disp, const WCHAR *name); typedef struct _builtin_prop_t builtin_prop_t; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11211