[PATCH v11 0/1] MR10464: vbscript: Return "Name redefined" error for duplicate declarations.
When a name is defined twice (Const/Const, Dim/Const, Const/Dim, Dim/Dim, Sub vs Const/Dim, Class vs Const/Dim/Function, duplicate class members), Windows returns VBScript error 1041 ("Name redefined"). Wine was returning E_FAIL. Store source locations in const_decl_t, dim_decl_t, function_decl_t, and class_decl_t so errors point to the duplicate name. Also fix duplicate Sub/Function/Property/Dim detection inside classes, including the case where a Property collides with an existing Sub/Function. -- v11: vbscript: Return "Name redefined" error for duplicate declarations. https://gitlab.winehq.org/wine/wine/-/merge_requests/10464
From: Francis De Brabandere <francisdb@gmail.com> Only check for function name collisions when compiling at global scope. Local Dim inside a Sub/Function/Property should not conflict with global functions of the same name, matching Windows behavior. --- dlls/vbscript/compile.c | 66 +++++++++++++- dlls/vbscript/interp.c | 4 +- dlls/vbscript/parse.h | 3 + dlls/vbscript/parser.y | 36 +++++--- dlls/vbscript/tests/lang.vbs | 56 ++++++++++++ dlls/vbscript/tests/run.c | 167 +++++++++++++++++++++++++++++++++++ 6 files changed, 313 insertions(+), 19 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 6ef01832c47..476a3cd4f75 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -432,6 +432,18 @@ static expression_t *lookup_const_decls(compile_ctx_t *ctx, const WCHAR *name, B return NULL; } +static const_decl_t *find_const_decl(compile_ctx_t *ctx, const WCHAR *name) +{ + const_decl_t *decl; + + for(decl = ctx->const_decls; decl; decl = decl->next) { + if(!vbs_wcsicmp(decl->name, name)) + return decl; + } + + return NULL; +} + static BOOL lookup_args_name(compile_ctx_t *ctx, const WCHAR *name) { unsigned i; @@ -456,6 +468,18 @@ static BOOL lookup_dim_decls(compile_ctx_t *ctx, const WCHAR *name) return FALSE; } +static BOOL lookup_func_decls(compile_ctx_t *ctx, const WCHAR *name) +{ + function_decl_t *func_decl; + + for(func_decl = ctx->func_decls; func_decl; func_decl = func_decl->next) { + if(!vbs_wcsicmp(func_decl->name, name)) + return TRUE; + } + + return FALSE; +} + static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *ret) { unsigned arg_cnt = 0; @@ -1203,11 +1227,21 @@ static HRESULT compile_dim_statement(compile_ctx_t *ctx, dim_statement_t *stat) while(1) { if(lookup_dim_decls(ctx, dim_decl->name) || lookup_args_name(ctx, dim_decl->name) - || lookup_const_decls(ctx, dim_decl->name, FALSE)) { + || (ctx->func->type == FUNC_GLOBAL && lookup_func_decls(ctx, dim_decl->name))) { ctx->loc = dim_decl->loc; + WARN("dim %s name redefined\n", debugstr_w(dim_decl->name)); return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } + { + const_decl_t *const_decl = find_const_decl(ctx, dim_decl->name); + if(const_decl) { + ctx->loc = dim_decl->loc > const_decl->loc ? dim_decl->loc : const_decl->loc; + WARN("dim %s name redefined\n", debugstr_w(dim_decl->name)); + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + } + } + ctx->func->var_cnt++; if(dim_decl->is_array) { @@ -1282,10 +1316,18 @@ static HRESULT compile_const_statement(compile_ctx_t *ctx, const_statement_t *st if(!lookup_const_decls(ctx, decl->name, FALSE)) { if(lookup_args_name(ctx, decl->name) || lookup_dim_decls(ctx, decl->name)) { + ctx->loc = decl->loc; + WARN("%s redefined\n", debugstr_w(decl->name)); return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } } + if(lookup_func_decls(ctx, decl->name)) { + ctx->loc = decl->loc; + WARN("%s redefined\n", debugstr_w(decl->name)); + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + } + if(ctx->func->type == FUNC_GLOBAL) { HRESULT hres; @@ -1448,10 +1490,15 @@ static HRESULT collect_const_decls(compile_ctx_t *ctx, statement_t *stat) for(decl = const_stat->decls; decl; decl = decl->next) { const_decl_t *new_decl; - if(lookup_const_decls(ctx, decl->name, FALSE)) - break; /* already collected */ + if(lookup_const_decls(ctx, decl->name, FALSE)) { + ctx->loc = decl->loc; + WARN("%s: const redefined\n", debugstr_w(decl->name)); + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); + } if(lookup_args_name(ctx, decl->name) || lookup_dim_decls(ctx, decl->name)) { + ctx->loc = decl->loc; + WARN("%s redefined\n", debugstr_w(decl->name)); return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } @@ -1459,6 +1506,7 @@ static HRESULT collect_const_decls(compile_ctx_t *ctx, statement_t *stat) if(!new_decl) return E_OUTOFMEMORY; new_decl->name = decl->name; + new_decl->loc = decl->loc; new_decl->value_expr = decl->value_expr; new_decl->next = ctx->const_decls; ctx->const_decls = new_decl; @@ -1789,6 +1837,8 @@ static HRESULT create_function(compile_ctx_t *ctx, function_decl_t *decl, functi HRESULT hres; if(lookup_dim_decls(ctx, decl->name) || lookup_const_decls(ctx, decl->name, FALSE)) { + ctx->loc = decl->loc; + WARN("%s: redefinition\n", debugstr_w(decl->name)); return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } @@ -1911,6 +1961,8 @@ static HRESULT compile_class(compile_ctx_t *ctx, class_decl_t *class_decl) if(lookup_dim_decls(ctx, class_decl->name) || lookup_funcs_name(ctx, class_decl->name) || lookup_const_decls(ctx, class_decl->name, FALSE) || lookup_class_name(ctx, class_decl->name)) { + ctx->loc = class_decl->loc; + WARN("%s: redefinition\n", debugstr_w(class_decl->name)); return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } @@ -1983,6 +2035,14 @@ static HRESULT compile_class(compile_ctx_t *ctx, class_decl_t *class_decl) for(prop_decl = class_decl->props, i=0; prop_decl; prop_decl = prop_decl->next, i++) { if(lookup_class_funcs(class_desc, prop_decl->name)) { + function_decl_t *func_iter; + unsigned loc = prop_decl->loc; + for(func_iter = class_decl->funcs; func_iter; func_iter = func_iter->next) { + if(!vbs_wcsicmp(func_iter->name, prop_decl->name) && func_iter->loc > loc) + loc = func_iter->loc; + } + WARN("%s: redefined\n", debugstr_w(prop_decl->name)); + ctx->loc = loc; return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 4bbdad181e0..e57f893e7a9 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1099,8 +1099,8 @@ static HRESULT interp_const(exec_ctx_t *ctx) return hres; if(ref.type != REF_NONE) { - FIXME("%s already defined\n", debugstr_w(arg)); - return E_FAIL; + WARN("%s already defined\n", debugstr_w(arg)); + return MAKE_VBSERROR(VBSE_NAME_REDEFINED); } hres = stack_assume_val(ctx, 0); diff --git a/dlls/vbscript/parse.h b/dlls/vbscript/parse.h index 271122b59ae..2af4e23d103 100644 --- a/dlls/vbscript/parse.h +++ b/dlls/vbscript/parse.h @@ -199,6 +199,7 @@ typedef struct _arg_decl_t { typedef struct _function_decl_t { const WCHAR *name; + unsigned loc; function_type_t type; BOOL is_public; BOOL is_default; @@ -215,6 +216,7 @@ typedef struct { typedef struct _class_decl_t { const WCHAR *name; + unsigned loc; function_decl_t *funcs; dim_decl_t *props; struct _class_decl_t *next; @@ -264,6 +266,7 @@ typedef struct { typedef struct _const_decl_t { const WCHAR *name; + unsigned loc; expression_t *value_expr; struct _const_decl_t *next; } const_decl_t; diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y index a7d95eaa707..f22c8bdfe7c 100644 --- a/dlls/vbscript/parser.y +++ b/dlls/vbscript/parser.y @@ -67,9 +67,9 @@ static dim_decl_t *new_dim_decl(parser_ctx_t*,const WCHAR*,unsigned,BOOL,dim_lis static dim_list_t *new_dim(parser_ctx_t*,unsigned,dim_list_t*); static redim_decl_t *new_redim_decl(parser_ctx_t*,const WCHAR*,expression_t*); static elseif_decl_t *new_elseif_decl(parser_ctx_t*,unsigned,expression_t*,statement_t*); -static function_decl_t *new_function_decl(parser_ctx_t*,const WCHAR*,function_type_t,unsigned,arg_decl_t*,statement_t*); +static function_decl_t *new_function_decl(parser_ctx_t*,unsigned,const WCHAR*,function_type_t,unsigned,arg_decl_t*,statement_t*); static arg_decl_t *new_argument_decl(parser_ctx_t*,const WCHAR*,BOOL); -static const_decl_t *new_const_decl(parser_ctx_t*,const WCHAR*,expression_t*); +static const_decl_t *new_const_decl(parser_ctx_t*,unsigned,const WCHAR*,expression_t*); static case_clausule_t *new_case_clausule(parser_ctx_t*,expression_t*,statement_t*,case_clausule_t*); static class_decl_t *new_class_decl(parser_ctx_t*); @@ -297,7 +297,7 @@ ConstDeclList | ConstDecl ',' ConstDeclList { $1->next = $3; $$ = $1; } ConstDecl - : Identifier '=' ConstExpression { $$ = new_const_decl(ctx, $1, $3); CHECK_ERROR; } + : Identifier '=' ConstExpression { $$ = new_const_decl(ctx, @1, $1, $3); CHECK_ERROR; } ConstExpression : LiteralExpression { $$ = $1; } @@ -480,7 +480,7 @@ PrimaryExpression | tME { $$ = new_expression(ctx, EXPR_ME, 0); CHECK_ERROR; } ClassDeclaration - : tCLASS Identifier StSep ClassBody tEND tCLASS StSep { $4->name = $2; $$ = $4; } + : tCLASS Identifier StSep ClassBody tEND tCLASS StSep { $4->name = $2; $4->loc = @2; $$ = $4; } ClassBody : /* empty */ { $$ = new_class_decl(ctx); } @@ -495,21 +495,21 @@ ClassBody PropertyDecl : Storage_opt tPROPERTY tGET Identifier ArgumentsDecl_opt StSep BodyStatements tEND tPROPERTY - { $$ = new_function_decl(ctx, $4, FUNC_PROPGET, $1, $5, $7); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @4, $4, FUNC_PROPGET, $1, $5, $7); CHECK_ERROR; } | Storage_opt tPROPERTY tLET Identifier '(' ArgumentDeclList ')' StSep BodyStatements tEND tPROPERTY - { $$ = new_function_decl(ctx, $4, FUNC_PROPLET, $1, $6, $9); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @4, $4, FUNC_PROPLET, $1, $6, $9); CHECK_ERROR; } | Storage_opt tPROPERTY tSET Identifier '(' ArgumentDeclList ')' StSep BodyStatements tEND tPROPERTY - { $$ = new_function_decl(ctx, $4, FUNC_PROPSET, $1, $6, $9); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @4, $4, FUNC_PROPSET, $1, $6, $9); CHECK_ERROR; } FunctionDecl : Storage_opt tSUB Identifier StSep BodyStatements tEND tSUB - { $$ = new_function_decl(ctx, $3, FUNC_SUB, $1, NULL, $5); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @3, $3, FUNC_SUB, $1, NULL, $5); CHECK_ERROR; } | Storage_opt tSUB Identifier ArgumentsDecl Nl_opt BodyStatements tEND tSUB - { $$ = new_function_decl(ctx, $3, FUNC_SUB, $1, $4, $6); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @3, $3, FUNC_SUB, $1, $4, $6); CHECK_ERROR; } | Storage_opt tFUNCTION Identifier StSep BodyStatements tEND tFUNCTION - { $$ = new_function_decl(ctx, $3, FUNC_FUNCTION, $1, NULL, $5); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @3, $3, FUNC_FUNCTION, $1, NULL, $5); CHECK_ERROR; } | Storage_opt tFUNCTION Identifier ArgumentsDecl Nl_opt BodyStatements tEND tFUNCTION - { $$ = new_function_decl(ctx, $3, FUNC_FUNCTION, $1, $4, $6); CHECK_ERROR; } + { $$ = new_function_decl(ctx, @3, $3, FUNC_FUNCTION, $1, $4, $6); CHECK_ERROR; } Storage_opt : /* empty*/ { $$ = 0; } @@ -1088,7 +1088,7 @@ static arg_decl_t *new_argument_decl(parser_ctx_t *ctx, const WCHAR *name, BOOL return arg_decl; } -static function_decl_t *new_function_decl(parser_ctx_t *ctx, const WCHAR *name, function_type_t type, +static function_decl_t *new_function_decl(parser_ctx_t *ctx, unsigned loc, const WCHAR *name, function_type_t type, unsigned storage_flags, arg_decl_t *arg_decl, statement_t *body) { function_decl_t *decl; @@ -1109,6 +1109,7 @@ static function_decl_t *new_function_decl(parser_ctx_t *ctx, const WCHAR *name, return NULL; decl->name = name; + decl->loc = loc; decl->type = type; decl->is_public = !(storage_flags & STORAGE_IS_PRIVATE); decl->is_default = is_default; @@ -1139,6 +1140,7 @@ static class_decl_t *new_class_decl(parser_ctx_t *ctx) if(!class_decl) return NULL; + class_decl->loc = 0; class_decl->funcs = NULL; class_decl->props = NULL; class_decl->next = NULL; @@ -1151,13 +1153,18 @@ static class_decl_t *add_class_function(parser_ctx_t *ctx, class_decl_t *class_d for(iter = class_decl->funcs; iter; iter = iter->next) { if(!wcsicmp(iter->name, decl->name)) { - if(decl->type == FUNC_SUB || decl->type == FUNC_FUNCTION) { + if(decl->type == FUNC_SUB || decl->type == FUNC_FUNCTION + || iter->type == FUNC_SUB || iter->type == FUNC_FUNCTION) { + WARN("%s::%s redefined\n", debugstr_w(class_decl->name), debugstr_w(decl->name)); + ctx->error_loc = iter->loc; ctx->hres = MAKE_VBSERROR(VBSE_NAME_REDEFINED); return NULL; } while(1) { if(iter->type == decl->type) { + WARN("%s::%s redefined\n", debugstr_w(class_decl->name), debugstr_w(decl->name)); + ctx->error_loc = iter->loc; ctx->hres = MAKE_VBSERROR(VBSE_NAME_REDEFINED); return NULL; } @@ -1198,7 +1205,7 @@ static class_decl_t *add_dim_prop(parser_ctx_t *ctx, class_decl_t *class_decl, d return class_decl; } -static const_decl_t *new_const_decl(parser_ctx_t *ctx, const WCHAR *name, expression_t *expr) +static const_decl_t *new_const_decl(parser_ctx_t *ctx, unsigned loc, const WCHAR *name, expression_t *expr) { const_decl_t *decl; @@ -1207,6 +1214,7 @@ static const_decl_t *new_const_decl(parser_ctx_t *ctx, const WCHAR *name, expres return NULL; decl->name = name; + decl->loc = loc; decl->value_expr = expr; decl->next = NULL; return decl; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index cce5b02e9cb..30b9c2cc35b 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -2423,6 +2423,36 @@ Class class_test_identifiers_as_property_name End Property End Class +' Local Dim inside a class method should not conflict with a global Function of the same name +Function GlobalShadowFunc() + GlobalShadowFunc = 42 +End Function + +Class TestLocalDimShadowsGlobalFunc + Public Sub TestShadow() + Dim GlobalShadowFunc + GlobalShadowFunc = 10 + Call ok(GlobalShadowFunc = 10, "local Dim should shadow global Function: " & GlobalShadowFunc) + End Sub + + Private Function ClassPrivateFunc() + ClassPrivateFunc = 99 + End Function + + Public Sub TestPrivate() + Call ok(ClassPrivateFunc() = 99, "private class function should be callable: " & ClassPrivateFunc()) + End Sub +End Class + +' Global function with same name as private class function +Function ClassPrivateFunc() + ClassPrivateFunc = 1 +End Function + +Dim objShadow : Set objShadow = New TestLocalDimShadowsGlobalFunc +objShadow.TestShadow +objShadow.TestPrivate + sub test_dotIdentifiers ' test keywords that can also be an identifier after a dot Call ok(testObj.rem = 10, "testObj.rem = " & testObj.rem & " expected 10") @@ -3083,6 +3113,32 @@ Call ok(Err.Number = 13, "Eval type mismatch: Err.Number = " & Err.Number & " ex Call ok(Err.Source = "Microsoft VBScript runtime error", "Eval type mismatch: Err.Source = """ & Err.Source & """") On Error Goto 0 +' Duplicate Sub declarations: last one wins +dim dupSubResult +dupSubResult = 0 +Sub DupSub() + dupSubResult = 1 +End Sub +Sub DupSub() + dupSubResult = 2 +End Sub +DupSub +call ok(dupSubResult = 2, "dupSubResult = " & dupSubResult) + +' Sub replaced by Function with same name: Function wins +dim dupMixedResult +dupMixedResult = 0 +Sub DupMixed() + dupMixedResult = 1 +End Sub +Function DupMixed() + dupMixedResult = 2 + DupMixed = 42 +End Function +dim dupMixedRet +dupMixedRet = DupMixed() +call ok(dupMixedResult = 2, "dupMixedResult = " & dupMixedResult) +call ok(dupMixedRet = 42, "dupMixedRet = " & dupMixedRet) ' Test calling a dispatch variable as statement (invokes default property) funcCalled = "" diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index 70ccd517254..71cdb2c6bb2 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -3079,6 +3079,173 @@ static void test_parse_errors(void) L"\x00e9var = 1\n", 0, 0, NULL, S_OK, 1032 + }, + { + /* Const redefined */ + L"Const X = 1\n" + "Const X = 2\n", + 1, 6, + L"Const X = 2", S_OK, 1041 + }, + { + /* Const redefined on same statement */ + L"Const C = 1, C = 2\n", + 0, 13, + L"Const C = 1, C = 2", S_OK, 1041 + }, + { + /* Dim after Const with same name */ + L"Const D = 1\n" + "Dim D\n", + 1, 4, + L"Dim D", S_OK, 1041 + }, + { + /* Const after Dim with same name */ + L"Dim E\n" + "Const E = 1\n", + 1, 6, + L"Const E = 1", S_OK, 1041 + }, + { + /* Sub after Const with same name */ + L"Const F = 1\n" + "Sub F()\n" + "End Sub\n", + 1, 4, + L"Sub F()", S_OK, 1041 + }, + { + /* Sub after Dim with same name */ + L"Dim G\n" + "Sub G()\n" + "End Sub\n", + 1, 4, + L"Sub G()", S_OK, 1041 + }, + { + /* Function after Const with same name */ + L"Const H = 1\n" + "Function H()\n" + "End Function\n", + 1, 9, + L"Function H()", S_OK, 1041 + }, + { + /* Const after Sub with same name */ + L"Sub I()\n" + "End Sub\n" + "Const I = 1\n", + 2, 6, + L"Const I = 1", S_OK, 1041 + }, + { + /* Dim after Sub with same name */ + L"Sub J()\n" + "End Sub\n" + "Dim J\n", + 2, 4, + L"Dim J", S_OK, 1041 + }, + { + /* Class after Const with same name */ + L"Const K = 1\n" + "Class K\n" + "End Class\n", + 1, 6, + L"Class K", S_OK, 1041 + }, + { + /* Class after Dim with same name */ + L"Dim L\n" + "Class L\n" + "End Class\n", + 1, 6, + L"Class L", S_OK, 1041 + }, + { + /* Class after Sub with same name */ + L"Sub M()\n" + "End Sub\n" + "Class M\n" + "End Class\n", + 2, 6, + L"Class M", S_OK, 1041 + }, + { + /* Duplicate Sub inside Class */ + L"Class N\n" + " Sub Foo\n" + " End Sub\n" + " Sub Foo\n" + " End Sub\n" + "End Class\n", + 3, 8, + L" Sub Foo", S_OK, 1041 + }, + { + /* Duplicate Function inside Class */ + L"Class O\n" + " Function Foo\n" + " End Function\n" + " Function Foo\n" + " End Function\n" + "End Class\n", + 3, 13, + L" Function Foo", S_OK, 1041 + }, + { + /* Sub and Function with same name inside Class */ + L"Class P\n" + " Sub Foo\n" + " End Sub\n" + " Function Foo\n" + " End Function\n" + "End Class\n", + 3, 13, + L" Function Foo", S_OK, 1041 + }, + { + /* Property Get and Sub with same name inside Class */ + L"Class Q\n" + " Property Get Foo\n" + " End Property\n" + " Sub Foo\n" + " End Sub\n" + "End Class\n", + 3, 8, + L" Sub Foo", S_OK, 1041 + }, + { + /* Duplicate Property Get inside Class */ + L"Class R\n" + " Property Get Foo\n" + " End Property\n" + " Property Get Foo\n" + " End Property\n" + "End Class\n", + 3, 17, + L" Property Get Foo", S_OK, 1041 + }, + { + /* Dim and Sub with same name inside Class */ + L"Class S\n" + " Dim Foo\n" + " Sub Foo\n" + " End Sub\n" + "End Class\n", + 2, 8, + L" Sub Foo", S_OK, 1041 + }, + { + /* Sub and Dim with same name inside Class */ + L"Class T\n" + " Sub Foo\n" + " End Sub\n" + " Dim Foo\n" + "End Class\n", + 3, 8, + L" Dim Foo", S_OK, 1041 } }; HRESULT hres; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10464
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)