[PATCH v3 0/1] MR4615: vbscript: For for loop coerce right bound string to real.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55052 -- v3: vbscript: For for loop bounds coerce string to real. https://gitlab.winehq.org/wine/wine/-/merge_requests/4615
From: Robert Wilhelm <robert.wilhelm(a)gmx.net> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55052 --- dlls/vbscript/compile.c | 6 ++++-- dlls/vbscript/interp.c | 32 ++++++++++++++++++++++++++++++++ dlls/vbscript/tests/lang.vbs | 27 +++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 1 + 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 683c71e2f4e..baddc51d7e4 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -875,6 +875,8 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st hres = compile_expression(ctx, stat->from_expr); if(FAILED(hres)) return hres; + if(!push_instr(ctx, OP_numval)) + return E_OUTOFMEMORY; /* FIXME: Assign should happen after both expressions evaluation. */ instr = push_instr(ctx, OP_assign_ident); @@ -887,7 +889,7 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(FAILED(hres)) return hres; - if(!push_instr(ctx, OP_val)) + if(!push_instr(ctx, OP_numval)) return E_OUTOFMEMORY; if(stat->step_expr) { @@ -895,7 +897,7 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(FAILED(hres)) return hres; - if(!push_instr(ctx, OP_val)) + if(!push_instr(ctx, OP_numval)) return E_OUTOFMEMORY; }else { hres = push_instr_int(ctx, OP_int, 1); diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 2d979f07605..bbe5ca7a6b2 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1088,6 +1088,38 @@ static HRESULT interp_val(exec_ctx_t *ctx) return stack_push(ctx, val.owned ? val.v : &v); } +static HRESULT interp_numval(exec_ctx_t *ctx) +{ + variant_val_t val; + VARIANT v,vcoerced,*r; + HRESULT hres; + BOOL hastocoerce = FALSE; + + TRACE("\n"); + + hres = stack_pop_val(ctx, &val); + if(FAILED(hres)) + return hres; + + r = val.v; + if(!val.owned) { + V_VT(&v) = VT_EMPTY; + hres = VariantCopy(&v, val.v); + if(FAILED(hres)) + return hres; + r = &v; + } + + if (V_VT(r) == VT_BSTR) { + hastocoerce = TRUE; + V_VT(&vcoerced) = VT_EMPTY; + hres = VariantChangeType(&vcoerced, r, 0, VT_R8); + if(FAILED(hres)) + return hres; + } + return stack_push(ctx, hastocoerce? &vcoerced : r); +} + static HRESULT interp_pop(exec_ctx_t *ctx) { const unsigned n = ctx->instr->arg1.uint; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 42af6df0e76..3c6ce656f1c 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -679,6 +679,33 @@ for x = 5 to 8 next Call ok(y = "for8: 5 7", "y = " & y) +function testfor( startvalue, endvalue, stepvalue, steps) + Dim s + s=0 + for x=startvalue to endvalue step stepvalue + s = s + 1 + Next + Call ok( s = steps, "counted " & s & " steps in for loop, expected " & steps) +end function + +Call testfor (1, 2, 1, 2) +Call testfor ("1", 2, 1, 2) +Call testfor (1, "2", 1, 2) +Call testfor (1, 2, "1", 2) +Call testfor ("1", "2", "1", 2) +if (isEnglishLang) then + Call testfor (1, 2, 0.5, 3) + Call testfor (1, 2.5, 0.5, 4) + Call testfor ("1", 2, 0.5, 3) + Call testfor ("1", 2.5, 0.5, 4) + Call testfor (1, "2", 0.5, 3) + Call testfor (1, "2.5", 0.5, 4) + Call testfor (1, 2, "0.5", 3) + Call testfor (1, 2.5, "0.5", 4) + Call testfor ("1", "2", "0.5", 3) + Call testfor ("1", "2.5", "0.5", 4) +end if + for x = 1.5 to 1 Call ok(false, "for..to called when unexpected") next diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 24915a5ca04..56e68411f45 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -278,6 +278,7 @@ typedef enum { X(not, 1, 0, 0) \ X(nothing, 1, 0, 0) \ X(null, 1, 0, 0) \ + X(numval, 1, 0, 0) \ X(or, 1, 0, 0) \ X(pop, 1, ARG_UINT, 0) \ X(redim, 1, ARG_BSTR, ARG_UINT) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4615
Hi Jacek, thanks for review. I added new opcode "numval" and it seems to work fine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4615#note_55356
Jacek Caban (@jacek) commented about dlls/vbscript/interp.c:
+ if(!val.owned) { + V_VT(&v) = VT_EMPTY; + hres = VariantCopy(&v, val.v); + if(FAILED(hres)) + return hres; + r = &v; + } + + if (V_VT(r) == VT_BSTR) { + hastocoerce = TRUE; + V_VT(&vcoerced) = VT_EMPTY; + hres = VariantChangeType(&vcoerced, r, 0, VT_R8); + if(FAILED(hres)) + return hres; + } + return stack_push(ctx, hastocoerce? &vcoerced : r); This looks correct now, but you could avoid extra copy and simplify things a bit if you handled VT_BSTR first, separately. You could move `VT_BSTR` check earlier, pass `val.v` as `VariantChangeType` source, call `release_val` and return early with `stack_push`. The rest of the function could then look just like `interp_val`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4615#note_55385
participants (3)
-
Jacek Caban (＠jacek) -
Robert Wilhelm
-
Robert Wilhelm (＠sloper42)