Re: [PATCH 01/10] vbscript: Fixed CBool(try 3)

[Shuai Meng]

2014-04-21 17:18 GMT+08:00 Jacek Caban :
> Hi Shuai,
>
> On 04/21/14 03:45, Shuai Meng wrote:
> > -
> > - return return_bool(res, val);
> > - V_VT(res) = VT_EMPTY;
> > - return VariantChangeType(res, arg, VARIANT_LOCALBOOL, VT_BOOL);
>
> You can't assume that res is not NULL. If result of the function is not
> used, it will be NULL. A simple test case is like this:
>
> call CBool(0)
>
> Also, are you sure we want VARIANT_LOCALBOOL here? Did you test it
> (probably by running tests on localized Windows with translated string
> "False")?
>
> Looking at CBool tests:
>
> +Call ok(CBool(Empty) = False, "CBool(Empty) = " & CBool(Empty))
> +Call ok(getVT(CBool(Empty)) = "VT_BOOL", "getVT(CBool(Empty)) = " &
> getVT(CBool(Empty)))
> +Call ok(CBool(1) = True, "CBool(1) = " & CBool(1))
> +Call ok(getVT(CBool(1)) = "VT_BOOL", "getVT(CBool(1)) = " &
> getVT(CBool(1)))
> +Call ok(CBool(CLng(0)) = False, "CBool(CLng(0)) = " & CBool(CLng(0)))
> +Call ok(getVT(CBool(CLng(0))) = "VT_BOOL", "getVT(CBool(CLng(0))) = " &
> getVT(CBool(CLng(0))))
> +Call ok(CBool(CSng(0.5)) = True, "CBool(CSng(0.5)) = " & CBool(CSng(0.5)))
> +Call ok(getVT(CBool(CSng(0.5))) = "VT_BOOL", "getVT(CBool(CSng(0.5))) = "
> & getVT(CBool(CSng(0.5))))
> +Call ok(CBool(-0.56) = True, "CBool(-0.56) = " & CBool(-0.56))
> +Call ok(getVT(CBool(-0.56)) = "VT_BOOL", "getVT(CBool(-0.56)) = " &
> getVT(CBool(-0.56)))
> +Call ok(CBool(CCur(8.55)) = True, "CBool(CCur(8.55)) = " &
> CBool(CCur(8.55)))
> +Call ok(getVT(CBool(CCur(8.55))) = "VT_BOOL", "getVT(CBool(CCur(8.55))) =
> " & getVT(CBool(CCur(8.55))))
> +Call ok(CBool(CDate(0)) = False, "CBool(CDate(0)) = " & CBool(CDate(0)))
> +Call ok(getVT(CBool(CDate(0))) = "VT_BOOL", "getVT(CBool(CDate(0))) = " &
> getVT(CBool(CDate(0))))
> +Call ok(CBool(CDate(3.33)) = True, "CBool(CDate(3.33)) = " &
> CBool(CDate(3.33)))
> +Call ok(getVT(CBool(CDate(3.33))) = "VT_BOOL", "getVT(CBool(CDate(3.33)))
> = " & getVT(CBool(CDate(3.33))))
> +Call ok(CBool("6666.78") = True, "CBool(""6666.78"") = " &
> CBool("6666.78"))
> +Call ok(getVT(CBool("6666.78")) = "VT_BOOL", "getVT(CBool(""6666.78"")) =
> " & getVT(CBool("6666.78")))
> +Call ok(CBool(CByte(-0.1)) = False, "CBool(CByte(-0.1)) = " &
> CBool(CByte(-0.1)))
> +Call ok(getVT(CBool(CByte(-0.1))) = "VT_BOOL", "getVT(CBool(CByte(-0.1)))
> = " & getVT(CBool(CByte(-0.1))))
>
> Those look mostly good. However I'd like to see some more tests.
> Interesting case is for example empty string and a "false", "False" and
> "#FALSE#" string.

These strings can't pass tests in testbot, I have tried. And I find a line
in http://msdn.microsoft.com/en-us/library/2k9sfx3c(v=vs.84).aspx  says
that "If *expression* can't be interpreted as a numeric value, a run-time
error occurs."

Also I'd really like to see tests on object default
> values. See the attached patch for an example.

I copied the example, and found that though tests on object default values
passed in testbot, they can't pass in wine. I think this is because
VariantChangeType function can't change an object into a VT_BOOL. So I
delete them.
> Those comments apply also to other patches in the series. It may be
> easier to proceed if you resent just the first patch fixed and include
> tests that don't depending on other patches in the series in the patch
> itself.
>
> Cheers,
> Jacek