Re: [PATCH] vbscript: Fixed CBool

[Piotr Caban]

On 04/29/14 15:06, Shuai Meng wrote:
> - if(!res)
> -
>    return S_OK;

You can't return early if res is NULL. You still need to do the 
conversion and return error if it fails.
> +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(0) = False, "CBool(0) = " & CBool(0))
> +Call ok(getVT(CBool(0)) = "VT_BOOL", "getVT(CBool(0)) = " & getVT(CBool(0)))
> +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("-1") = True, "CBool(""-1"") = " & CBool("-1"))
> +Call ok(getVT(CBool("-1")) = "VT_BOOL", "getVT(CBool(""-1"")) = " & getVT(CBool("-1")))

It would be nice to add following tests (these are the tests Jacek was 
asking for):
Call ok(CBool("True") = true, "CBool(""True"") = " & CBool("True"))
Call ok(CBool("fAlSe") = false, "CBool(""fAlSe"") = " & CBool("fAlSe"))
sub testCBoolError
     on error resume next

call Err.clear()
     call CBool("#FALSE#")
     call ok(Err.number = 458, "Err.number = " & Err.number)
end sub
Call testCBoolError

[PATCH] vbscript: Fixed CBool

[Shuai Meng]

Thank you for commenting.

2014-04-29 21:30 GMT+08:00 Piotr Caban :
> On 04/29/14 15:06, Shuai Meng wrote:
> > - if(!res)
> > -
> >    return S_OK;
>
> You can't return early if res is NULL. You still need to do the conversion
> and return error if it fails.

I  don't quite get it, do you mean I should do like this:
if(res)
{
....
}
return S_OK;
> +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(0) = False, "CBool(0) = " & CBool(0))
> > +Call ok(getVT(CBool(0)) = "VT_BOOL", "getVT(CBool(0)) = " &
> > getVT(CBool(0)))
> > +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("-1") = True, "CBool(""-1"") = " & CBool("-1"))
> > +Call ok(getVT(CBool("-1")) = "VT_BOOL", "getVT(CBool(""-1"")) = " &
> > getVT(CBool("-1")))
>
> It would be nice to add following tests (these are the tests Jacek was
> asking for):

Call ok(CBool("True") = true, "CBool(""True"") = " & CBool("True"))
> Call ok(CBool("fAlSe") = false, "CBool(""fAlSe"") = " & CBool("fAlSe"))

I do notice it, but they failed in testbot. Just as MSDN says, If
*expression* can't be interpreted as a numeric value, a run-time error
occurs.

sub testCBoolError
> on error resume next
>
> call Err.clear()
> call CBool("#FALSE#")
> call ok(Err.number = 458, "Err.number = " & Err.number)
>
> end sub
> Call testCBoolError
>
> Is this function written to provoke an error on purpose in order to test

CBool("#FALSE#")? Can CBool("True") assume the same form? Because
CBool("True") always fails in testbot as I have said.

[PATCH] vbscript: Fixed CBool

[Piotr Caban]

On 04/29/14 16:04, Shuai Meng wrote:
> Thank you for commenting.
>
> 2014-04-29 21:30 GMT+08:00 Piotr Caban <[e-mail]
> >:
> On 04/29/14 15:06, Shuai Meng wrote:
>
>     +    if(!res)
>     +        return S_OK;
>
> You can't return early if res is NULL. You still need to do the
> conversion and return error if it fails.
>
> I  don't quite get it, do you mean I should do like this:
> if(res)
> {
> ....
> }
> return S_OK;

No, I mean that you should do something like this:
VARIANT v;
...
V_VT(&v) = VT_EMPTY;
hr = VariantChangeType(..., &v);
if(FAILED(hr))
     return hr;
...
if(res)
     *res = v;
else
     VariantClear(&v);
> It would be nice to add following tests (these are the tests Jacek
> was asking for):
>
> Call ok(CBool("True") = true, "CBool(""True"") = " & CBool("True"))
> Call ok(CBool("fAlSe") = false, "CBool(""fAlSe"") = " & CBool("fAlSe"))
>
>
> I do notice it, but they failed in testbot. Just as MSDN says, If
> /expression/ can't be interpreted as a numeric value, a run-time error
> occurs.

You must have done something else incorrectly. Here's a run that 
succeeded: https://testbot.winehq.org/JobDetails.pl?Key=6665. It shows 
that CBool("True") returns true.
> sub testCBoolError
>      on error resume next
>
>      call Err.clear()
>      call CBool("#FALSE#")
>      call ok(Err.number = 458, "Err.number = " & Err.number)
> end sub
> Call testCBoolError
>
> Is this function written to provoke an error on purpose in order to test
> CBool("#FALSE#")?

It's meant to handle error returned by CBool("#FALSE#"). Other invalid 
parameters may be tested in the same way.

Cheers,
Piotr

[PATCH] vbscript: Fixed CBool

[Shuai Meng]

2014-04-29 22:25 GMT+08:00 Piotr Caban :
> On 04/29/14 16:04, Shuai Meng wrote:
> > Thank you for commenting.
> >
> > 2014-04-29 21:30 GMT+08:00 Piotr Caban <[e-mail]
> > >:
> > On 04/29/14 15:06, Shuai Meng wrote:
> >
> >     +    if(!res)
> >     +        return S_OK;
> >
> > You can't return early if res is NULL. You still need to do the
> > conversion and return error if it fails.
> >
> > I  don't quite get it, do you mean I should do like this:
> > if(res)
> > {
> > ....
> > }
> > return S_OK;
>
> No, I mean that you should do something like this:
> VARIANT v;
> ...
> V_VT(&v) = VT_EMPTY;
> hr = VariantChangeType(..., &v);
> if(FAILED(hr))
>     return hr;
> ...

if(res)
> *res = v;
>
> else
>     VariantClear(&v);

what will be returned after  *res =  v; or VariantClear(&v); ?

It would be nice to add following tests (these are the tests Jacek
> > was asking for):
> >
> > Call ok(CBool("True") = true, "CBool(""True"") = " & CBool("True"))
> > Call ok(CBool("fAlSe") = false, "CBool(""fAlSe"") = " &
> >
> > CBool("fAlSe"))
> >
> > I do notice it, but they failed in testbot. Just as MSDN says, If
> > /expression/ can't be interpreted as a numeric value, a run-time error
> > occurs.
>
> You must have done something else incorrectly. Here's a run that
> succeeded: https://testbot.winehq.org/JobDetails.pl?Key=6665. It shows
> that CBool("True") returns true.

yes, I made a mistake. The error occurs when I test an empty string as
Jacek have asked:
477 Call ok(CBool("") = True, "CBool("""") = " & CBool(""))
478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("""")) = " &
getVT(CBool("")))
I have tried to solve it like this:
477 Call ok(CBool("") = True, "CBool("") = " & CBool(""))
478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("")) = " &
getVT(CBool("")))
and this:
477 Call ok(CBool("") = True, "CBool(" & "" & ") = " & CBool(""))
478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool(" & "" & ")) = " &
getVT(CBool("")))
They all don't work...The problem is how describe an empty string between
two quotation marks.
>  sub testCBoolError
> >      on error resume next
> >
> >      call Err.clear()
> >      call CBool("#FALSE#")
> >      call ok(Err.number = 458, "Err.number = " & Err.number)
> > end sub
> > Call testCBoolError
> >
> > Is this function written to provoke an error on purpose in order to test
> > CBool("#FALSE#")?
>
> It's meant to handle error returned by CBool("#FALSE#"). Other invalid
> parameters may be tested in the same way.
>
> Cheers,
> Piot
> r

[PATCH] vbscript: Fixed CBool

[Piotr Caban]

On 04/29/14 17:25, Shuai Meng wrote:
> 2014-04-29 22:25 GMT+08:00 Piotr Caban <[e-mail]
> >:
>     No, I mean that you should do something like this:
>     VARIANT v;
>     ...
>     V_VT(&v) = VT_EMPTY;
>     hr = VariantChangeType(..., &v);
>     if(FAILED(hr))
>          return hr;
>     ...
> if(res)
>      *res = v;
> else
>      VariantClear(&v);
>
> what will be returned after  *res =  v; or VariantClear(&v); ?

S_OK, because the function has succeeded.
> yes, I made a mistake. The error occurs when I test an empty string as
> Jacek have asked:
> 477 Call ok(CBool("") = True, "CBool("""") = " & CBool(""))
> 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("""")) = " &
> getVT(CBool("")))
> I have tried to solve it like this:
> 477 Call ok(CBool("") = True, "CBool("") = " & CBool(""))
> 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("")) = " &
> getVT(CBool("")))
> and this:
> 477 Call ok(CBool("") = True, "CBool(" & "" & ") = " & CBool(""))
> 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool(" & "" & ")) = "
> & getVT(CBool("")))
> They all don't work...The problem is how describe an empty string
> between two quotation marks.

It's ok to use "" for empty string. The CBool is simply failing for that 
argument. It can be tested in similar way as "#FALSE# string.

[PATCH] vbscript: Fixed CBool

[Shuai Meng]

Thank you.

2014-04-30 1:13 GMT+08:00 Piotr Caban :
> On 04/29/14 17:25, Shuai Meng wrote:
> > 2014-04-29 22:25 GMT+08:00 Piotr Caban <[e-mail]
> > >:
> > No, I mean that you should do something like this:
> > VARIANT v;
> > ...
> > V_VT(&v) = VT_EMPTY;
> > hr = VariantChangeType(..., &v);
> > if(FAILED(hr))
> >      return hr;
> > ...
> >
> > if(res)
> >      *res = v;
> > else
> >      VariantClear(&v);
> >
> > what will be returned after  *res =  v; or VariantClear(&v); ?
>
> S_OK, because the function has succeeded.

Then I think this the same as return  VariantChangeType directly. How about
writing like this:

if(res)
{
    V_VT(res) = VT_EMPTY;
    return VariantChangeType(res, arg, 0, VT_BOOL);
}
return S_OK;

Because whether the change succeed or not, VariantChangeType will return a
value indicating the situation.
> yes, I made a mistake. The error occurs when I test an empty string as
> > Jacek have asked:
> > 477 Call ok(CBool("") = True, "CBool("""") = " & CBool(""))
> > 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("""")) = " &
> > getVT(CBool("")))
> > I have tried to solve it like this:
> > 477 Call ok(CBool("") = True, "CBool("") = " & CBool(""))
> > 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool("")) = " &
> > getVT(CBool("")))
> > and this:
> > 477 Call ok(CBool("") = True, "CBool(" & "" & ") = " & CBool(""))
> > 478 Call ok(getVT(CBool("")) = "VT_BOOL", "getVT(CBool(" & "" & ")) = "
> > & getVT(CBool("")))
> > They all don't work...The problem is how describe an empty string
> > between two quotation marks.
>
> It's ok to use "" for empty string. The CBool is simply failing for that
> argument. It can be tested in similar way as "#FALSE# string.

I got it. I will do like this. Thank you.

[PATCH] vbscript: Fixed CBool

[Jacek Caban]

On 04/30/14 02:42, Shuai Meng wrote:
> Thank you.
>
> 2014-04-30 1:13 GMT+08:00 Piotr Caban <[e-mail]
> >:
> On 04/29/14 17:25, Shuai Meng wrote:
>
>     2014-04-29 22:25 GMT+08:00 Piotr Caban <[e-mail]
>     <mailto:[e-mail]>
>     <mailto:[e-mail] <mailto:[e-mail]>>>:
>
>         No, I mean that you should do something like this:
>         VARIANT v;
>         ...
>         V_VT(&v) = VT_EMPTY;
>         hr = VariantChangeType(..., &v);
>         if(FAILED(hr))
>              return hr;
>         ...
>
>         if(res)
>              *res = v;
>         else
>              VariantClear(&v);
>
>       what will be returned after  *res =  v; or VariantClear(&v); ?
>
> S_OK, because the function has succeeded.
>
>
> Then I think this the same as return  VariantChangeType directly. How
> about writing like this:
>
> if(res)
> {
>     V_VT(res) = VT_EMPTY;
>     return VariantChangeType(res, arg, 0, VT_BOOL);
> }
> return S_OK;
>
> Because whether the change succeed or not, VariantChangeType will
> return a value indicating the situation.

Even if res is NULL (meaning that the return value is not used by the
script), the semantic of the code needs to be the same as if it was
used. It's just an optimization and optimizations can't change
semantics. For example:

call CBool("blah")

needs to return an error and you need to perform the conversion to
recognize the error. An other example why this is wrong is if you try to
convert an object. In such case, default value getter needs to be called
(and it's called by VariantChangeType in this case).

Jacek

[PATCH] vbscript: Fixed CBool

[Shuai Meng]

Thank you, I got it. And I'll correct it.

2014-04-30 16:05 GMT+08:00 Jacek Caban :
> On 04/30/14 02:42, Shuai Meng wrote:
>
> Thank you.
>
> 2014-04-30 1:13 GMT+08:00 Piotr Caban :
> > On 04/29/14 17:25, Shuai Meng wrote:
> > > 2014-04-29 22:25 GMT+08:00 Piotr Caban <[e-mail]
> > >  >:
> > > No, I mean that you should do something like this:
> > > VARIANT v;
> > > ...
> > > V_VT(&v) = VT_EMPTY;
> > > hr = VariantChangeType(..., &v);
> > > if(FAILED(hr))
> > >      return hr;
> > > ...
> > >
> > > if(res)
> > >      *res = v;
> > > else
> > >      VariantClear(&v);
> > >
> > > what will be returned after  *res =  v; or VariantClear(&v); ?
> >
> > S_OK, because the function has succeeded.
>
> Then I think this the same as return  VariantChangeType directly. How
> about writing like this:
>
> if(res)
> {
>     V_VT(res) = VT_EMPTY;
>     return VariantChangeType(res, arg, 0, VT_BOOL);
> }
> return S_OK;
>
> Because whether the change succeed or not, VariantChangeType will return
> a value indicating the situation.
>
> Even if res is NULL (meaning that the return value is not used by the
> script), the semantic of the code needs to be the same as if it was used.
> It's just an optimization and optimizations can't change semantics. For
> example:
>
> call CBool("blah")
>
> needs to return an error and you need to perform the conversion to
> recognize the error. An other example why this is wrong is if you try to
> convert an object. In such case, default value getter needs to be called
> (and it's called by VariantChangeType in this case).
>
> Jacek