[PATCH v24 0/1] MR10244: Draft: vbscript: Fix Sub first argument parentheses handling.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54177 Fix parsing of expressions like `Sub (i - 20) * 8,y` where a space-separated parenthesized expression followed by a binary operator(*` /` \` ^` &`) was incorrectly treated as call argument parentheses instead of expression grouping. If you look at all the extra test cases in `lang.vbs` you will see which parentheses related issues this solves. I have been testing with a repo full of scripts: https://github.com/sverrewl/vpxtable_scripts ~~There are still quite some compiler issues left but~~ this addresses the biggest parser issues. -- v24: vbscript: Fix Sub first argument parentheses handling. https://gitlab.winehq.org/wine/wine/-/merge_requests/10244
From: Francis De Brabandere <francisdb@gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54177 --- dlls/vbscript/lex.c | 107 +++++++++++++++++++++++- dlls/vbscript/parse.h | 2 + dlls/vbscript/parser.y | 16 +++- dlls/vbscript/tests/lang.vbs | 154 +++++++++++++++++++++++++++++++++++ 4 files changed, 273 insertions(+), 6 deletions(-) diff --git a/dlls/vbscript/lex.c b/dlls/vbscript/lex.c index 81cc67e037b..093a267484d 100644 --- a/dlls/vbscript/lex.c +++ b/dlls/vbscript/lex.c @@ -112,6 +112,27 @@ static inline BOOL is_identifier_char(WCHAR c) return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_'; } +/* Does [pos, end) start with `word` (ASCII case-insensitive) followed by a + * non-identifier char? Peek-only; does not advance anything. */ +static BOOL peek_keyword(const WCHAR *pos, const WCHAR *end, const WCHAR *word) +{ + WCHAR c; + + while(pos < end && *word) { + c = *pos; + if(c >= 'A' && c <= 'Z') c += 'a' - 'A'; + if(c != *word) + return FALSE; + pos++; + word++; + } + if(*word) + return FALSE; + if(pos < end && is_identifier_char(*pos)) + return FALSE; + return TRUE; +} + /* Compare the current parse position against a keyword using ASCII-only * case-insensitive matching. Keywords are all lowercase ASCII, so we only * need to lowercase [A-Z] in the source. Returns 0 on match, <0 or >0 @@ -509,7 +530,8 @@ static int parse_next_token(void *lval, unsigned *loc, parser_ctx_t *ctx) return comment_line(ctx); ctx->ptr++; return '-'; - case '(': + case '(': { + const WCHAR *paren_pos = ctx->ptr; /* NOTE: * We resolve empty brackets in lexer instead of parser to avoid complex conflicts * in call statement special case |f()| without 'call' keyword @@ -523,11 +545,51 @@ static int parse_next_token(void *lval, unsigned *loc, parser_ctx_t *ctx) /* * Parser can't predict if bracket is part of argument expression or an argument * in call expression. We predict it here instead. + * + * In statement context, 'f(x) OP y' treats (x) as expression grouping, + * not call parens. Detect this when: identifier/')' precedes '(', and a + * binary operator (punctuation or keyword) follows the matching ')'. + * '=' is excluded — 'f(x) = y' is parsed as assignment. */ if(ctx->last_token == tIdentifier || ctx->last_token == ')' || ctx->last_token == tME - || ctx->last_token == tEMPTYBRACKETS) + || ctx->last_token == tEMPTYBRACKETS) { + if(ctx->is_statement_ctx) { + const WCHAR *p = ctx->ptr; + int depth = 1; + + while(p < ctx->end && depth > 0) { + if(*p == '(') + depth++; + else if(*p == ')') + depth--; + if(depth > 0) + p++; + } + + if(depth == 0) { + p++; + while(p < ctx->end && (*p == ' ' || *p == '\t')) + p++; + if(p < ctx->end) { + if(*p == '*' || *p == '/' || *p == '\\' || *p == '^' || *p == '&' + || *p == '+' || *p == '-' || *p == '<' || *p == '>') + return tEXPRLBRACKET; + switch(*p | 0x20) { + case 'a': if(peek_keyword(p, ctx->end, L"and")) return tEXPRLBRACKET; break; + case 'e': if(peek_keyword(p, ctx->end, L"eqv")) return tEXPRLBRACKET; break; + case 'i': if(peek_keyword(p, ctx->end, L"imp") + || peek_keyword(p, ctx->end, L"is")) return tEXPRLBRACKET; break; + case 'm': if(peek_keyword(p, ctx->end, L"mod")) return tEXPRLBRACKET; break; + case 'o': if(peek_keyword(p, ctx->end, L"or")) return tEXPRLBRACKET; break; + case 'x': if(peek_keyword(p, ctx->end, L"xor")) return tEXPRLBRACKET; break; + } + } + } + } return '('; + } return tEXPRLBRACKET; + } case '[': return parse_bracket_identifier(ctx, lval); case '"': @@ -609,5 +671,46 @@ int parser_lex(void *lval, unsigned *loc, parser_ctx_t *ctx) } ctx->after_continuation = FALSE; + + /* Track paren depth and update is_statement_ctx only at depth 0. + * Tokens inside parentheses (e.g. the 'idx' in 'obj(idx).method') + * must not affect statement context tracking. */ + if(ret == '(' || ret == tEXPRLBRACKET) + ctx->paren_depth++; + else if(ret == ')' || ret == tEMPTYBRACKETS) { + if(ctx->paren_depth > 0) + ctx->paren_depth--; + } + + if(ctx->paren_depth == 0) { + switch(ret) { + case tNL: + case ':': + case tTHEN: + case tELSE: + case tELSEIF: + ctx->is_statement_ctx = TRUE; + break; + case tDOT: + case '.': + case ')': + case tEMPTYBRACKETS: + /* Dots and closing parens in member chains keep current state. */ + break; + case tIdentifier: + /* First identifier after statement start or after a dot keeps TRUE. + * An identifier after another identifier (e.g. 'Sub Arg') means + * we're in argument context, so set to FALSE. */ + if(ctx->last_token != '.' && ctx->last_token != tDOT + && ctx->last_token != tNL && ctx->last_token != ':' + && ctx->last_token != tTHEN && ctx->last_token != tELSE + && ctx->last_token != tELSEIF) + ctx->is_statement_ctx = FALSE; + break; + default: + ctx->is_statement_ctx = FALSE; + break; + } + } return (ctx->last_token = ret); } diff --git a/dlls/vbscript/parse.h b/dlls/vbscript/parse.h index a4d5079d10e..a181f814c0f 100644 --- a/dlls/vbscript/parse.h +++ b/dlls/vbscript/parse.h @@ -309,6 +309,8 @@ typedef struct { LCID lcid; int last_token; + BOOL is_statement_ctx; + int paren_depth; unsigned last_nl; BOOL after_continuation; diff --git a/dlls/vbscript/parser.y b/dlls/vbscript/parser.y index ea025dbd81c..750dc009c05 100644 --- a/dlls/vbscript/parser.y +++ b/dlls/vbscript/parser.y @@ -822,9 +822,13 @@ static call_expression_t *make_call_expression(parser_ctx_t *ctx, expression_t * return call_expr; if(arguments->type != EXPR_NOARG) { - FIXME("Invalid syntax: missing comma\n"); - ctx->hres = E_FAIL; - return NULL; + /* 'f (x) + expr, ...' — combine bracketed arg with the +/- expression. */ + expression_t *remaining = arguments->next; + call_expr->args = new_binary_expression(ctx, EXPR_ADD, call_expr->args, arguments); + if(!call_expr->args) + return NULL; + call_expr->args->next = remaining; + return call_expr; } call_expr->args->next = arguments->next; @@ -1307,14 +1311,18 @@ HRESULT parse_script(parser_ctx_t *ctx, const WCHAR *code, const WCHAR *delimite ctx->hres = S_OK; ctx->error_loc = -1; ctx->last_token = tNL; + ctx->is_statement_ctx = TRUE; + ctx->paren_depth = 0; ctx->last_nl = 0; ctx->stats = ctx->stats_tail = NULL; ctx->class_decls = NULL; ctx->option_explicit = FALSE; ctx->is_html = delimiter && !wcsicmp(delimiter, L"</script>"); - if(flags & SCRIPTTEXT_ISEXPRESSION) + if(flags & SCRIPTTEXT_ISEXPRESSION) { ctx->last_token = tEXPRESSION; + ctx->is_statement_ctx = FALSE; + } parser_parse(ctx); diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 7ff524461c6..343002c3929 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -1217,6 +1217,160 @@ Call TestSubExit2 TestSubMultiArgs 1, 2, 3, 4, 5 Call TestSubMultiArgs(1, 2, 3, 4, 5) +Sub TestSubParenExpr(a, b) + Call ok(a=16, "a = " & a) + Call ok(b=7, "b = " & b) +End Sub + +TestSubParenExpr (2) * 8, 7 +TestSubParenExpr 8 * (2), 7 + +Sub TestSubParenExprAdd(a, b) + Call ok(a=6, "a = " & a) + Call ok(b=7, "b = " & b) +End Sub + +TestSubParenExprAdd (2) + 4, 7 +TestSubParenExprAdd 4 + (2), 7 + +Sub TestSubParenExprNoSpace(a) + Call ok(a=6, "a = " & a) +End Sub + +TestSubParenExprNoSpace(10 \ 2) + 1 + +' Regression test: function call with space before ( in expression context +' e.g. x = (CInt (2) + 1) * 3 must parse and evaluate correctly +x = CInt (2) + 1 +Call ok(x = 3, "CInt (2) + 1 = " & x) +x = (CInt (2) + 1) * 3 +Call ok(x = 9, "(CInt (2) + 1) * 3 = " & x) + +' Regression test: function call with space before ( and * in expression context +' e.g. x = CInt (2) * 3 must treat CInt (2) as a function call, not expression grouping +x = CInt (2) * 3 +Call ok(x = 6, "CInt (2) * 3 = " & x) + +' Test member expression in statement context: obj.Method (x) * y, z +Class TestObjParenExpr + Sub Check(a, b) + Call ok(a=16, "obj a = " & a) + Call ok(b=7, "obj b = " & b) + End Sub +End Class + +Dim objParenExpr +Set objParenExpr = New TestObjParenExpr +objParenExpr.Check (2) * 8, 7 + +Sub TestSubParenExprConcat(a, b) + Call ok(a="helloworld", "a = " & a) + Call ok(b=7, "b = " & b) +End Sub + +TestSubParenExprConcat ("hello") & "world", 7 + +' Test: function call as argument with & after paren must be a call, not grouping +' e.g. TestSub Mid ("hello", 2) & "x" should call TestSub with "ellox" +' Mid("hello", 2) returns "ello", & "x" concatenates to "ellox" +Sub TestSubArgCallConcat(a) + Call ok(a="ellox", "a = " & a) +End Sub + +TestSubArgCallConcat Mid ("hello", 2) & "x" + +' Test: obj(idx).method (expr) * val, y in statement context +' The (expr) after .method must be expression grouping, not call paren +Class TestIndexedObjParenExpr + Public arr_(1) + Public Sub Init() + Set arr_(0) = New TestObjParenExpr + End Sub + Public Default Property Get Item(idx) + Set Item = arr_(idx) + End Property +End Class + +Dim idxObj +Set idxObj = New TestIndexedObjParenExpr +Call idxObj.Init() +idxObj(0).Check (2) * 8, 7 + +' No-space variants of Sub-first-arg paren pattern. +' On native VBScript, `S(x) OP y` in statement context treats the whole +' `S(x) OP y` as a call to S with argument `(x) OP y` — for every binary +' operator except `=` (parsed as assignment). +' Each case is wrapped in Execute so parse failures of one don't abort the rest. +Dim npArg, npArgA, npArgB +Sub NpS(a) + npArg = a +End Sub +Sub NpT(a, b) + npArgA = a + npArgB = b +End Sub + +Sub CheckNpS(src, expected) + npArg = Empty + On Error Resume Next + Err.Clear + Execute src + Dim e : e = Err.Number + On Error GoTo 0 + Call ok(e = 0, "parse error for " & src & ": err=" & e) + If e = 0 Then Call ok(npArg = expected, src & ": npArg = " & npArg & " expected " & expected) +End Sub + +CheckNpS "NpS(10)+5", 15 +CheckNpS "NpS(10)-3", 7 +CheckNpS "NpS(10)*3", 30 +CheckNpS "NpS(10)/2", 5 +CheckNpS "NpS(10)\3", 3 +CheckNpS "NpS(10)^2", 100 +CheckNpS "NpS(""hi"")&""!""", "hi!" +CheckNpS "NpS(10) Mod 3", 1 +CheckNpS "NpS(10)<>10", False +CheckNpS "NpS(10)<5", False +CheckNpS "NpS(10)>5", True +CheckNpS "NpS(10)<=10", True +CheckNpS "NpS(10)>=5", True +CheckNpS "NpS(1) And 1", 1 +CheckNpS "NpS(0) Or 1", 1 +CheckNpS "NpS(1) Xor 1", 0 +CheckNpS "NpS(1) Eqv 1", -1 +CheckNpS "NpS(1) Imp 1", -1 +CheckNpS "NpS(Nothing) Is Nothing", True + +' Two-arg form: S(x) OP y, z — result of `(x) OP y` is first arg, z is second. +Sub CheckNpT(src, expectedA, expectedB) + npArgA = Empty + npArgB = Empty + On Error Resume Next + Err.Clear + Execute src + Dim e : e = Err.Number + On Error GoTo 0 + Call ok(e = 0, "parse error for " & src & ": err=" & e) + If e = 0 Then Call ok(npArgA = expectedA and npArgB = expectedB, _ + src & ": a=" & npArgA & " b=" & npArgB) +End Sub + +CheckNpT "NpT(10)+5, 7", 15, 7 +CheckNpT "NpT(10)*3, 7", 30, 7 +CheckNpT "NpT(""hi"")&""!"", 7", "hi!", 7 + +' Member expression: obj.Method(x) OP y — no space, same pattern. +Class NpCls + Sub Check(a) + npArg = a + End Sub +End Class +Dim npObj +Set npObj = New NpCls +CheckNpS "npObj.Check(10)+5", 15 +CheckNpS "npObj.Check(10)*3", 30 + + Sub TestSubLocalVal x = false Call ok(not x, "local x is not false?") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10244
Fixed but even a bit uglier now with all the parser look-ahead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10244#note_136712
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)