[ros-dev] [ros-diffs] [akhaldi] 65249: [SHELL32] * bool => BOOL.
Love Nystrom
love.nystrom at gmail.com
Mon Nov 10 05:32:42 UTC 2014
On 2014-11-10 05.19, Timo Kreuzer wrote:
> The point is: It is different. A C++ bool will always be only 1 or 0.
> MSVC does actually warn (a performance warning) when assigning an int
> to a bool, GCC does not warn. And it's ok, since it will (should) do
> the right thing.
I'm glad GCC have the good sense to not complain ;)
That's what I wrote the bool_cast function to deal with in MSVC...
so you don't have to litter your code with warning disabling pragmas.
> A bool will always be 0 or 1. A BOOL could be anything. So if there is
> code relying on a parameter to be 0 or 1, since it was a bool, it
> might fail, when it gets a BOOL, which might be 0x80000000.
> "if (FooBar == TRUE)" might fail, if FooBar is converted from bool to
> BOOL.
I agree..
I strongly favor bool, which is better than BOOL for a few reasons I can
think of.
1) It's *safe*, unlike BOOL, even for explicit truth-value compares
(which are truly bad habit).
The following will work with bool, but fail with BOOL:
volatile DWORD Flags = 0xFFFFFFFF;
bool ok = bool_cast( Flags & 0x80000000 );
if (ok == TRUE) puts( "Passed" ); else puts( "Failed" ); // Passed
BOOL B52 = Flags & 0x80000000;
if (B52 == TRUE) puts( "Passed" ); else puts( "Failed" ); // Failed
2) The compiler can (potentially) generate more efficient binary code
for bool.
> It might not be an issue. And I don't say we should not use BOOL or
> BOOLEAN, I just want to point out that these are not the same thing
> and you cannot just convert one to the other and expect that
> everything works as before.
It *is* a potential issue, but perhaps an unexpected one..
We *must forbid* explicit truth-value comparisons, since they are
bombers, per the above.
(The "logical not" operator (!) exists for a reason.. I'm sure we can
all agree on that. ;)
On 2014-11-10 07.56, David Quintana (gigaherz) wrote:
> All the common COM interfaces use BOOL instead of bool, because it's
> more compiler-agnostic and it matches better the Win32 API type usage.
> If we use bool, we may risk a "consumer" of the interface using the
> wrong size for "bool", so although ISFHelper is an internal interface
> not meant to be used by 3rdparty programs, BOOL just fits better. It
> wasn't wrong before, it was just "out of place" kinda.
>
The compiler will always push a 32 bit boolean value, regardless if its
bool or BOOL,
so it should not be a concern as far as the size is concerned.
More importantly, as Timo originally stated, and I illuminated above,
changing bool to BOOL can potentially introduce code malfunctions,
which I'm sure none of us would like.
Best Regards
// Love
>
>
> Am 09.11.2014 17:28, schrieb Love Nystrom:
>> @Timo, @Amine
>>
>> A BOOL should merely distinguish between zero and non-zero, so
>> extending an 8 bit bool
>> (which I favor) to a 32 bit BOOL should always produce a valid
>> result, even in the example case.
>> If Foo becomes true after the mask test, it is expressed as non-zero,
>> which persist through the
>> BOOLEAN, and still remain non-zero when expanded to BOOL, so either
>> should be OK,
>> provided you don't need to comply with someone elses interface.
>> As arguments, the compiler will always pass 32 bit in either case
>> (afaik).
>>
>> But MSVC *will* whine about assigning FlagsValue & 0x80000000 to a bool.
>> I made a bool_cast function (below) to deal with those cases, since
>> programmers
>> usually know what they do when they choose to squeeze a BOOL into a
>> bool.
>> true == TRUE, which we know, but the compiler whines and bitches about.
>> I'm not sure how to deal with gcc for this, but it ought be similar.
>>
>> Masking 0x80000000 directly to BOOLEAN will fail of course, which the
>> compiler
>> detects: conversion from 'DWORD' to 'BOOLEAN', possible loss of data
>>
>> #ifdef _MSC_VER
>> // This cast is forced inline, and should not generate any extra code,
>> // just coax the compiler to treat the BOOL as a bool without whining.
>>
>> #pragma warning( disable: 4800 )
>> bool __forceinline bool_cast( BOOL value ) {
>> return (bool) value;
>> }
>> #pragma warning( default: 4800 )
>> #endif
>>
>> Then you are free to do:
>>
>> bool Foo = bool_cast( Flags & 0x80000000 ); // Nice, quiet, and
>> compact ;-)
>>
>> Use it carefully though.
>>
>> Just my penny to the bucket..
>> Best Regards
>> // Love
>>
>> On 2014-11-05 05.06, Timo Kreuzer wrote:
>>>
>>> There is a difference between a C++ bool and a win32 BOOL. bool is 8
>>> bit and aware of things like (bool Foo = FlagsValue & 0x80000000);
>>> BOOLEAN BooleanFlag = Foo;" with BOOL this would fail. I don't
>>> really know whether a BOOL is really the right thing to use here.
>>>
>>> Am 04.11.2014 21:00, schrieb akhaldi at svn.reactos.org:
>>>> Author: akhaldi
>>>> Date: Tue Nov 4 20:00:09 2014
>>>> New Revision: 65249
>>>>
>>>> URL: http://svn.reactos.org/svn/reactos?rev=65249&view=rev
>>>> Log:
>>>> [SHELL32]
>>>> * bool => BOOL.
>>>>
>>>> Modified:
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.cpp
>>>>
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.h
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.cpp
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.h
>>>> branches/shell-experiments/dll/win32/shell32/shellfolder.h
>>>>
>>>> Modified:
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.cpp
>>>>
>>>> URL:
>>>> http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.cpp?rev=65249&r1=65248&r2=65249&view=diff
>>>> ==============================================================================
>>>>
>>>> ---
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.cpp
>>>> [iso-8859-1] (original)
>>>> +++
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.cpp
>>>> [iso-8859-1] Tue Nov 4 20:00:09 2014
>>>> @@ -1189,7 +1189,7 @@
>>>> return ret;
>>>> }
>>>> -HRESULT WINAPI CDesktopFolder::CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, bool bCopy)
>>>> +HRESULT WINAPI CDesktopFolder::CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, BOOL bCopy)
>>>> {
>>>> CComPtr<IPersistFolder2> ppf2;
>>>> WCHAR szSrcPath[MAX_PATH];
>>>>
>>>> Modified:
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.h
>>>> URL:
>>>> http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.h?rev=65249&r1=65248&r2=65249&view=diff
>>>> ==============================================================================
>>>>
>>>> ---
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.h
>>>> [iso-8859-1] (original)
>>>> +++
>>>> branches/shell-experiments/dll/win32/shell32/folders/CDesktopFolder.h
>>>> [iso-8859-1] Tue Nov 4 20:00:09 2014
>>>> @@ -88,7 +88,7 @@
>>>> virtual HRESULT WINAPI GetUniqueName(LPWSTR pwszName,
>>>> UINT uLen);
>>>> virtual HRESULT WINAPI AddFolder(HWND hwnd, LPCWSTR
>>>> pwszName, LPITEMIDLIST *ppidlOut);
>>>> virtual HRESULT WINAPI DeleteItems(UINT cidl,
>>>> LPCITEMIDLIST *apidl);
>>>> - virtual HRESULT WINAPI CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, bool bCopy);
>>>> + virtual HRESULT WINAPI CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, BOOL bCopy);
>>>> DECLARE_REGISTRY_RESOURCEID(IDR_SHELLDESKTOP)
>>>> DECLARE_NOT_AGGREGATABLE(CDesktopFolder)
>>>>
>>>> Modified:
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.cpp
>>>> URL:
>>>> http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.cpp?rev=65249&r1=65248&r2=65249&view=diff
>>>> ==============================================================================
>>>>
>>>> ---
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.cpp
>>>> [iso-8859-1] (original)
>>>> +++
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.cpp
>>>> [iso-8859-1] Tue Nov 4 20:00:09 2014
>>>> @@ -1030,7 +1030,7 @@
>>>> * copies items to this folder
>>>> */
>>>> HRESULT WINAPI CFSFolder::CopyItems(IShellFolder * pSFFrom, UINT
>>>> cidl,
>>>> - LPCITEMIDLIST * apidl, bool
>>>> bCopy)
>>>> + LPCITEMIDLIST * apidl, BOOL
>>>> bCopy)
>>>> {
>>>> CComPtr<IPersistFolder2> ppf2 = NULL;
>>>> WCHAR szSrcPath[MAX_PATH];
>>>>
>>>> Modified:
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.h
>>>> URL:
>>>> http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.h?rev=65249&r1=65248&r2=65249&view=diff
>>>> ==============================================================================
>>>>
>>>> ---
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.h
>>>> [iso-8859-1] (original)
>>>> +++
>>>> branches/shell-experiments/dll/win32/shell32/folders/CFSFolder.h
>>>> [iso-8859-1] Tue Nov 4 20:00:09 2014
>>>> @@ -102,7 +102,7 @@
>>>> virtual HRESULT WINAPI GetUniqueName(LPWSTR pwszName,
>>>> UINT uLen);
>>>> virtual HRESULT WINAPI AddFolder(HWND hwnd, LPCWSTR
>>>> pwszName, LPITEMIDLIST *ppidlOut);
>>>> virtual HRESULT WINAPI DeleteItems(UINT cidl,
>>>> LPCITEMIDLIST *apidl);
>>>> - virtual HRESULT WINAPI CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, bool bCopy);
>>>> + virtual HRESULT WINAPI CopyItems(IShellFolder *pSFFrom,
>>>> UINT cidl, LPCITEMIDLIST *apidl, BOOL bCopy);
>>>> DECLARE_REGISTRY_RESOURCEID(IDR_SHELLFSFOLDER)
>>>> DECLARE_NOT_AGGREGATABLE(CFSFolder)
>>>>
>>>> Modified: branches/shell-experiments/dll/win32/shell32/shellfolder.h
>>>> URL:
>>>> http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/shell32/shellfolder.h?rev=65249&r1=65248&r2=65249&view=diff
>>>> ==============================================================================
>>>>
>>>> --- branches/shell-experiments/dll/win32/shell32/shellfolder.h
>>>> [iso-8859-1] (original)
>>>> +++ branches/shell-experiments/dll/win32/shell32/shellfolder.h
>>>> [iso-8859-1] Tue Nov 4 20:00:09 2014
>>>> @@ -42,7 +42,7 @@
>>>> STDMETHOD(GetUniqueName)(THIS_ LPWSTR lpName, UINT uLen) PURE;
>>>> STDMETHOD(AddFolder)(THIS_ HWND hwnd, LPCWSTR lpName,
>>>> LPITEMIDLIST * ppidlOut) PURE;
>>>> STDMETHOD(DeleteItems)(THIS_ UINT cidl, LPCITEMIDLIST *
>>>> apidl) PURE;
>>>> - STDMETHOD(CopyItems)(THIS_ IShellFolder * pSFFrom, UINT cidl,
>>>> LPCITEMIDLIST * apidl, bool bCopy) PURE;
>>>> + STDMETHOD(CopyItems)(THIS_ IShellFolder * pSFFrom, UINT cidl,
>>>> LPCITEMIDLIST * apidl, BOOL bCopy) PURE;
>>>> };
>>>> #undef INTERFACE
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> Ros-dev mailing list
>> Ros-dev at reactos.org
>> http://www.reactos.org/mailman/listinfo/ros-dev
>>
>
>
>
More information about the Ros-dev
mailing list