Warning C4800 when casting from python bool to c bool

[Czarek Tomczak]

Hi,

I have this warning message when compiling:

    warning C4800: 'int' : forcing value to bool 'true' or 'false'

Here is the .pxd file:

    from libcpp cimport bool as c_bool

    void Find(int identifier, CefString& searchText, c_bool forward, c_bool matchCase, c_bool findNext)

The .pyx file:

    

    from cpython cimport bool as py_bool 

    cpdef object DoFind(self, int searchId, str searchText, py_bool forward, py_bool matchCase, py_bool findNext):

        Find(searchId, cefSearchText, forward, matchCase, findNext)

I can only get rid of this warning by changing the Find() call to:

    Find(searchId, cefSearchText, bool(forward), bool(matchCase), bool(findNext))

Should I add these bool() casts every time, or should I add compiler flag to ignore such warnings?

Czarek.

[Czarek Tomczak]

There is already a `py_string` type declared in function parameters, so why the need for calling bool()?

Czarek.

[Stefan Behnel]

Czarek Tomczak, 06.12.2012 23:44:

> I have this warning message when compiling:
>
> warning C4800: 'int' : forcing value to bool 'true' or 'false'

What C(++) compiler are you using? (And, just for completeness, which
version of Cython?)

It would also help if you pasted the relevant lines of the generated C file
when reporting C/C++ compiler errors and warnings.

> Here is the .pxd file:
>
> from libcpp cimport bool as c_bool
> void Find(int identifier, CefString& searchText, c_bool forward, c_bool
> matchCase, c_bool findNext)

Surprisingly enough, the declaration of 'bool' in libcpp/__init__.pxd is this:

ctypedef bint bool

That doesn't look right. Have you tried using plain 'bool' instead of
cimporting it from libcpp? AFAIR, that's the way to do it in C++ mode.

Maybe we should just delete the above declaration?

> The .pyx file:
>
> from cpython cimport bool as py_bool
> cpdef object DoFind(self, int searchId, str searchText, py_bool
> forward, py_bool matchCase, py_bool findNext):
> Find(searchId, cefSearchText, forward, matchCase, findNext)

I have no idea why you do this. Enforcing Python's bool type here makes
your interface very unpythonic.

> I can only get rid of this warning by changing the Find() call to:
>
> Find(searchId, cefSearchText, bool(forward), bool(matchCase),
> bool(findNext))
>
> Should I add these bool() casts every time

Definitely not. Python-to-C++-bool coercion should just work.

> or should I add compiler flag to ignore such warnings?

I've never seen these myself, but I think fixing the declarations should work.

Stefan

[Czarek Tomczak]

On Thursday, December 6, 2012 10:11:59 PM UTC-8, Stefan Behnel wrote:

What C(++) compiler are you using? (And, just for completeness, which
version of Cython?)

I'm using Visual Studio 2008 SP1 compiler & Cython 0.17.2.

 

It would also help if you pasted the relevant lines of the generated C file
when reporting C/C++ compiler errors and warnings.

cefpython.cpp(8597) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)

Line 8597:

    

    __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

Lines before:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":210

    * cdef CefString cefSearchText

    * ToCefString(searchText, cefSearchText)

    * self.GetCefBrowser().get().Find(searchId, cefSearchText, forward,             # <<<<<<<<<<<<<<

    *                                matchCase, findNext)

    * 

    */

   __pyx_t_4 = ((struct __pyx_vtabstruct_14cefpython_py27_PyBrowser *)__pyx_v_self->__pyx_vtab)->GetCefBrowser(__pyx_v_self); if (unlikely(PyErr_Occurred())) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

    __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;} // <---------------- line 8597

 

Surprisingly enough, the declaration of 'bool' in libcpp/__init__.pxd is this:

    ctypedef bint bool

That doesn't look right. Have you tried using plain 'bool' instead of
cimporting it from libcpp? AFAIR, that's the way to do it in C++ mode.

I'm not sure what you mean by plain bool, there is no such type identifier, I need to import it from cpython or libcpp, if by plain bool you mean the one from cpython, then I've tried this:

    from cpython cimport bool as py_bool

    from cpython cimport bool as c_bool

But now getting a different warning:

    cefpython.cpp(8602) : warning C4800: 'PyBoolObject *' : forcing value to bool 'true' or 'false' (performance warning)

Line 8602:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":211

     * ToCefString(searchText, cefSearchText)

     * self.GetCefBrowser().get().Find(searchId, cefSearchText, forward,

     *                                matchCase, findNext)             # <<<<<<<<<<<<<<

     * 

     * cpdef PyFrame GetFocusedFrame(self):

     */

    __pyx_t_4.get()->Find(__pyx_v_searchId, __pyx_v_cefSearchText, __pyx_v_forward, __pyx_v_matchCase, __pyx_v_findNext);

 

> The .pyx file: 
>     
>     from cpython cimport bool as py_bool 
>     cpdef object DoFind(self, int searchId, str searchText, py_bool 
> forward, py_bool matchCase, py_bool findNext): 
>         Find(searchId, cefSearchText, forward, matchCase, findNext) 

I have no idea why you do this. Enforcing Python's bool type here makes 
your interface very unpythonic. 

I'm not sure what you mean. I'm just making functions signatures strict typed to catch mistakes early.

 

Czarek.

[Czarek Tomczak]

On Thursday, December 6, 2012 10:11:59 PM UTC-8, Stefan Behnel wrote:

What C(++) compiler are you using? (And, just for completeness, which
version of Cython?)

Detailed compiler info:

    Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.21022.08 for 80x86

    Copyright (C) Microsoft Corporation.  All rights reserved. 

Czarek.

[Stefan Behnel]

Czarek Tomczak, 07.12.2012 08:27:

> On Thursday, December 6, 2012 10:11:59 PM UTC-8, Stefan Behnel wrote:
>
>> What C(++) compiler are you using? (And, just for completeness, which
>> version of Cython?)
>
> I'm using Visual Studio 2008 SP1 compiler & Cython 0.17.2.
>
>
>> It would also help if you pasted the relevant lines of the generated C
>> file when reporting C/C++ compiler errors and warnings.
>
> cefpython.cpp(8597) : warning C4800: 'int' : forcing value to bool 'true'
> or 'false' (performance warning)
>
> Line 8597:
>
> __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if

> (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {...}

Hmm - this is wrong. It shouldn't try to cast -1 to a C++ bool. That's
exactly the kind of broken code that the declaration of 'bool' in
libcpp/__init__.pxd would give you. I think we should remove it.

>> Surprisingly enough, the declaration of 'bool' in libcpp/__init__.pxd is
>> this:
>>
>> ctypedef bint bool
>>
>> That doesn't look right. Have you tried using plain 'bool' instead of
>> cimporting it from libcpp? AFAIR, that's the way to do it in C++ mode.
>
> I'm not sure what you mean by plain bool, there is no such type identifier,

Hmm, doesn't this work?

cdef bool b

*without* cimporting from libcpp?

> I need to import it from cpython or libcpp, if by plain bool you mean the
> one from cpython, then I've tried this:
>
> from cpython cimport bool as py_bool
> from cpython cimport bool as c_bool
>
> But now getting a different warning:
>
> cefpython.cpp(8602) : warning C4800: 'PyBoolObject *' : forcing value
> to bool 'true' or 'false' (performance warning)
>
> Line 8602:
>
> /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":211

> * self.GetCefBrowser().get().Find(searchId, cefSearchText, forward,
> * matchCase, findNext) #

> */
> __pyx_t_4.get()->Find(__pyx_v_searchId, __pyx_v_cefSearchText,
> __pyx_v_forward, __pyx_v_matchCase, __pyx_v_findNext);

Ok, this looks completely broken now.

>>> The .pyx file:
>>>
>>> from cpython cimport bool as py_bool
>>> cpdef object DoFind(self, int searchId, str searchText, py_bool
>>> forward, py_bool matchCase, py_bool findNext):
>>> Find(searchId, cefSearchText, forward, matchCase, findNext)
>>
>> I have no idea why you do this. Enforcing Python's bool type here makes
>> your interface very unpythonic.
>
> I'm not sure what you mean. I'm just making functions signatures strict
> typed to catch mistakes early.

That's ok for most types, but it's uncommon for boolean values and strings.
In Python, [] evaluates to False just like 0 or None. If you enforce the
Python 'bool' type at the interface level, you force users to explicitly
pass in True or False, instead of whatever boolean-compatible value they
currently have in their hands. I.e. code like this will fail with a TypeError:

x = 1
if x:
DoFind(1, u'abc', forward=x, matchCase=None, findNext=False)

although it would work perfectly in normal Python code. In Py2, your code
will reject the unicode text value and in all Python versions it will
reject the non-bool values for forward and matchCase in this example.

Instead, test (and, if necessary, convert) the value of searchText
yourself, and use the C++ bool type for the boolean values to get
auto-conversion on input.

That's also the reason why 'bool' isn't the Python bool type in C++ mode -
it's simply useless in almost all cases. What you want in the 99% case is
the C++ bool type.

Stefan

[Stefan Behnel]

Stefan Behnel, 07.12.2012 09:03:

> Hmm, doesn't this work?
>
> cdef bool b
>
> *without* cimporting from libcpp?

Hmm, tried it now, doesn't work. So I think we have to somehow either fix
the declaration in libcpp/__init__.pxd or provide a proper declaration as
part of the built-in compiler type system.

Stefan

[Robert Bradshaw]

On Fri, Dec 7, 2012 at 12:03 AM, Stefan Behnel <stef...@behnel.de> wrote:
>
> Czarek Tomczak, 07.12.2012 08:27:
> > On Thursday, December 6, 2012 10:11:59 PM UTC-8, Stefan Behnel wrote:
> >
> >> What C(++) compiler are you using? (And, just for completeness, which
> >> version of Cython?)
> >
> > I'm using Visual Studio 2008 SP1 compiler & Cython 0.17.2.
> >
> >
> >> It would also help if you pasted the relevant lines of the generated C
> >> file when reporting C/C++ compiler errors and warnings.
> >
> > cefpython.cpp(8597) : warning C4800: 'int' : forcing value to bool 'true'
> > or 'false' (performance warning)
> >
> > Line 8597:
> >
> > __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if
> > (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {...}
>
> Hmm - this is wrong. It shouldn't try to cast -1 to a C++ bool. That's
> exactly the kind of broken code that the declaration of 'bool' in
> libcpp/__init__.pxd would give you. I think we should remove it.

What would you suggest instead? (Declaring it as an int would have
exactly the same issue, but not print as nicely or accept as many
values.) I suppose we could build knowledge of C++ bool into the
compiler... Suppressing the warning is totally safe as the code is
perfectly correct (though admittedly not optimal).

- Robert

[Bradley Froehle]

I've found that pretending that the C++ bool is a `bint` in Cython works just fine in almost every case.

[Bradley Froehle]

> >     __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if
> > (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {...}
>
> Hmm - this is wrong. It shouldn't try to cast -1 to a C++ bool. That's
> exactly the kind of broken code that the declaration of 'bool' in
> libcpp/__init__.pxd would give you. I think we should remove it.

What would you suggest instead? (Declaring it as an int would have
exactly the same issue, but not print as nicely or accept as many
values.) I suppose we could build knowledge of C++ bool into the
compiler... Suppressing the warning is totally safe as the code is
perfectly correct (though admittedly not optimal).

Yes, the code is certainly valid.  The only non-optimal aspect, as far as I can tell, is the call to PyErr_Occurred whenever the object evaluates to true.  (Recall that (bool)-1 == true).

-Brad

[Stefan Behnel]

Robert Bradshaw, 07.12.2012 09:21:

> On Fri, Dec 7, 2012 at 12:03 AM, Stefan Behnel wrote:
>> Czarek Tomczak, 07.12.2012 08:27:

>>> cefpython.cpp(8597) : warning C4800: 'int' : forcing value to bool 'true'
>>> or 'false' (performance warning)
>>>
>>> Line 8597:
>>>
>>> __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if
>>> (unlikely((__pyx_t_5 == (bool)-1) && PyErr_Occurred())) {...}
>>
>> Hmm - this is wrong. It shouldn't try to cast -1 to a C++ bool. That's
>> exactly the kind of broken code that the declaration of 'bool' in
>> libcpp/__init__.pxd would give you. I think we should remove it.
>
> What would you suggest instead? (Declaring it as an int would have
> exactly the same issue, but not print as nicely or accept as many
> values.) I suppose we could build knowledge of C++ bool into the
> compiler... Suppressing the warning is totally safe as the code is
> perfectly correct (though admittedly not optimal).

I was thinking that we should let the compiler know about C++ bool
explicitly, but if a simple alias for bint works for everyone, I'm fine
with just suppressing the warning (or letting users do so).

Stefan

[Czarek Tomczak]

On Friday, December 7, 2012 12:03:50 AM UTC-8, Stefan Behnel wrote:

> I'm not sure what you mean. I'm just making functions signatures strict 

> typed to catch mistakes early.

That's ok for most types, but it's uncommon for boolean values and strings.
In Python, [] evaluates to False just like 0 or None. If you enforce the
Python 'bool' type at the interface level, you force users to explicitly
pass in True or False, instead of whatever boolean-compatible value they
currently have in their hands. I.e. code like this will fail with a TypeError:

    x = 1
    if x:
        DoFind(1, u'abc', forward=x, matchCase=None, findNext=False)

although it would work perfectly in normal Python code. In Py2, your code
will reject the unicode text value and in all Python versions it will
reject the non-bool values for forward and matchCase in this example.

Instead, test (and, if necessary, convert) the value of searchText
yourself, and use the C++ bool type for the boolean values to get
auto-conversion on input.

That's also the reason why 'bool' isn't the Python bool type in C++ mode -
it's simply useless in almost all cases. What you want in the 99% case is
the C++ bool type.

Okay, you're right about the "str", I've added py_string type so I can still see that this is a "string" type in function signature:

    # We should allow multiple string types: str, unicode, bytes. ToCefString() can 

    # handle them all.

    ctypedef object py_string

Regarding the bools, that is the behavior I want, to strictly demand a bool, because you can pass an array by mistake and makes it a hard to find bug.

I still don't see the right solution for the libcpp.bool/cpython.bool, using only libcpp.bool also gets me many warnings, I think I will go with both cpython.bool as py_bool / libcpp.bool and explicitily cast using bool() when passing to a C++ function, it removes any warnings:

    cpdef object Find(self, int searchId, py_string searchText, py_bool forward,

              py_bool matchCase, py_bool findNext):

        self.GetCefBrowser().get().Find(searchId, cefSearchText, bool(forward),

                                                    bool(matchCase), bool(findNext))

The C++ code generated by this is:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":210

     * cdef CefString cefSearchText

     * ToCefString(searchText, cefSearchText)

     * self.GetCefBrowser().get().Find(searchId, cefSearchText, bool(forward),             # <<<<<<<<<<<<<<

     *                                bool(matchCase), bool(findNext))

     * 

     */

  __pyx_t_4 = ((struct __pyx_vtabstruct_14cefpython_py27_PyBrowser *)__pyx_v_self->__pyx_vtab)->GetCefBrowser(__pyx_v_self); if (unlikely(PyErr_Occurred())) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

  __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if (unlikely(__pyx_t_5 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

Is this code fine? If yes, then why not generate such code by default without requiring a call to bool()?

Czarek.

[Czarek Tomczak]

On Friday, December 7, 2012 8:52:06 AM UTC-8, Czarek Tomczak wrote:

.....

The C++ code generated by this is:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":210

     * cdef CefString cefSearchText

     * ToCefString(searchText, cefSearchText)

     * self.GetCefBrowser().get().Find(searchId, cefSearchText, bool(forward),             # <<<<<<<<<<<<<<

     *                                bool(matchCase), bool(findNext))

     * 

     */

  __pyx_t_4 = ((struct __pyx_vtabstruct_14cefpython_py27_PyBrowser *)__pyx_v_self->__pyx_vtab)->GetCefBrowser(__pyx_v_self); if (unlikely(PyErr_Occurred())) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

  __pyx_t_5 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_forward)); if (unlikely(__pyx_t_5 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 210; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

Is this code fine? If yes, then why not generate such code by default without requiring a call to bool()?

There seems to be more code:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":211

    * ToCefString(searchText, cefSearchText)

    * self.GetCefBrowser().get().Find(searchId, cefSearchText, bool(forward),

    *                                bool(matchCase), bool(findNext))             # <<<<<<<<<<<<<<

    * 

    * cpdef PyFrame GetFocusedFrame(self):

    */

    __pyx_t_6 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_matchCase)); if (unlikely(__pyx_t_6 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 211; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

    __pyx_t_7 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_findNext)); if (unlikely(__pyx_t_7 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 211; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

    __pyx_t_4.get()->Find(__pyx_v_searchId, __pyx_v_cefSearchText, (!(!__pyx_t_5)), (!(!__pyx_t_6)), (!(!__pyx_t_7)));

    __pyx_r = Py_None; __Pyx_INCREF(Py_None);

    goto __pyx_L0;

    __pyx_L1_error:;

    __Pyx_XDECREF(__pyx_t_1);

    __Pyx_XDECREF(__pyx_t_2);

    __Pyx_XDECREF(__pyx_t_3);

    __Pyx_AddTraceback("cefpython_py27.PyBrowser.Find", __pyx_clineno, __pyx_lineno, __pyx_filename);

    __pyx_r = 0;

    __pyx_L0:;

    __Pyx_XGIVEREF(__pyx_r);

    __Pyx_RefNannyFinishContext();

    return __pyx_r;

  } 

Czarek.

[Czarek Tomczak]

On Friday, December 7, 2012 8:57:51 AM UTC-8, Czarek Tomczak wrote:

    /* "C:\cefpython\cefpython\cefpython\cef1\windows\setup\browser.pyx":211

    * ToCefString(searchText, cefSearchText)

    * self.GetCefBrowser().get().Find(searchId, cefSearchText, bool(forward),

    *                                bool(matchCase), bool(findNext))             # <<<<<<<<<<<<<<

    * 

    * cpdef PyFrame GetFocusedFrame(self):

    */

    __pyx_t_6 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_matchCase)); if (unlikely(__pyx_t_6 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 211; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

    __pyx_t_7 = __Pyx_PyObject_IsTrue(((PyObject *)__pyx_v_findNext)); if (unlikely(__pyx_t_7 < 0)) {__pyx_filename = __pyx_f[5]; __pyx_lineno = 211; __pyx_clineno = __LINE__; goto __pyx_L1_error;}

    __pyx_t_4.get()->Find(__pyx_v_searchId, __pyx_v_cefSearchText, (!(!__pyx_t_5)), (!(!__pyx_t_6)), (!(!__pyx_t_7)));

This is probably not performant acting, so maybe I should go with the warnings and ignore them via compiler flag? What I'm afraid of is that it might hide some other problems in the code in the future, not releated to this bool scenario, or is it not?

Czarek.

[Robert Bradshaw]

Can you not ignore just this one warning type?

- Robert

[Czarek Tomczak]

On Friday, December 7, 2012 10:14:46 AM UTC-8, Robert Bradshaw wrote:

...

Can you not ignore just this one warning type?

Yes I can ignore this specific warning C4800 by adding a compiler flag:

/ignore:4800

Or a pragma directive:

#pragma warning(disable: 4800)

My concern wasn't about ignoring all warnings, only this specific warning, it still might hide some other problems in my code, not related to this specific bool sceneario, so I'm not convinced this is a perfect way of doing this.

Czarek.