Hi,
The following code does not do what one would think:
sal_uInt8 nValue = 255;
Any aValue;
aValue <<= nValue;
aValue now contains a *BOOLEAN*. That's rather... surprising, and
could bite any of us one day. In particular:
sal_uInt8 naValue;
aValue >>= naValue;
now naValue contains 1. Not 255..
Ideally, I'd like that we fix this in LibreOffice 4.1.
The cleanest would be to add support for unsigned 8 bit integer in
Any, which as far as I understand would entail:
- adding typelib_TypeClass_UNSIGNED_BYTE to cppu/inc/typelib/typeclass.h
- adding "UNSIGNED_BYTE" to udkapi/com/sun/star/uno/TypeClass.idl (published!)
I'm not sure what the consequences of the latter are. E.g. do all Uno
bindings/bridges/... (Java, StarBasic, Python, ...) have to be updated
one by one?
Also, it seems the basic mechanism of dispatch with Any is C++
overloads; in particular, this is what happens in:
aValue <<= nValue
But sal_uInt8 and sal_Bool are typedefs of the same type, namely
"unsigned char", so I don't know how to make the distinction between
sal_Bool and sal_uInt8 in there.
So frankly, I'm not sure what to do... If we were a
pure C++ program, I'd change:
typedef unsigned char sal_Bool;
into
typedef bool sal_Bool;
or maybe
class sal_Bool
{
unsigned char m_bValue;
// constructors, member functions, operators, etc so that
// everything works right
}
But we have a "pure C" layer, so no can no do. In my understanding,
even C99's _Bool type won't save us, for several reasons:
1) I'm not sure it is really a "different type" than the other integer
types
2) It is not available in C++, so we can't have it in C++ code?
3) MSVC doesn't have C99 anyway
Hmm... Could we do something like:
typedef struct _sal_Bool
{
unsigned char bValue;
#ifdef __cplusplus
inline operator bool() const
{
return bValue;
}
inline _sal_Bool(bool b)
: bValue(b)
{ }
inline void operator=(bool b)
{
bValue=b;
}
#endif
} sal_Bool;
# define sal_False ((sal_Bool){0})
# define sal_True ((sal_Bool){1})
#ifdef __cplusplus
bool operator==(sal_Bool b0, sal_Bool b1)
{
return b0.bValue == b1.bValue;
}
#endif
??
So, in pure C code, we can still write:
sal_Bool b0 = sal_False;
sal_Bool b1 = sal_True;
b0 = b1;
but we'd have to write:
if (b0.bValue) {}
if (b0.bValue == b1.bValue) {}
b0.bValue = A || B && C
On the other hand, in C++ code, we could write:
if (b0 == sal_False) {}
if (b0 == b1) {}
if (b0) {}
b0 = A || B && C
The other solution would be an enum? Makes things a bit easier in C,
but less in C++. All these would work in C:
if (b0 == sal_False) {}
if (b0 == b1) {}
if (b0) {}
but not
b0 = A || B && C
and I don't think we can get it to work in C++, either.
So I'd rather make things maximally easy in C++, which is our main
language :)
Since we are in our "unstable API/ABI" period, *if* we can have a
clean solution, let's have it for LibreOffice 4.1. Since the in-memory
layout of sal_Bool does not change (right?), this might even be fully
ABI-compatible? (but not API-compatible for C, but API-compatible for
C++)
In terms of more hackish solutions, I have thought of not normalising
sal_Bool in Any (to make them "true" synonyms of sal_uInt8), *but*
this would break any code of the form:
if (b0 == sal_True) {}
switch (b0)
{
case sal_False:
case sal_True:
}
so, bleh, not looking forward to it. It would be something like:
diff --git a/cppu/inc/com/sun/star/uno/Any.hxx
b/cppu/inc/com/sun/star/uno/Any.hxx
index 6f3bda9..62ca967 100644
--- a/cppu/inc/com/sun/star/uno/Any.hxx
+++ b/cppu/inc/com/sun/star/uno/Any.hxx
@@ -248,7 +248,7 @@ inline sal_Bool SAL_CALL operator >>= ( const
::com::sun::star::uno::Any & rAny,
{
if (typelib_TypeClass_BOOLEAN == rAny.pType->eTypeClass)
{
- value = (* reinterpret_cast< const sal_Bool * >( rAny.pData ) !=
sal_False);
+ value = * reinterpret_cast< const sal_Bool * >( rAny.pData );
return sal_True;
}
return sal_False;
diff --git a/cppu/source/uno/copy.hxx b/cppu/source/uno/copy.hxx
index ce46492..d532d21 100644
--- a/cppu/source/uno/copy.hxx
+++ b/cppu/source/uno/copy.hxx
@@ -179,7 +179,7 @@ inline void _copyConstructAnyFromData(
break;
case typelib_TypeClass_BOOLEAN:
pDestAny->pData = &pDestAny->pReserved;
- *(sal_Bool *)pDestAny->pData = (*(sal_Bool *)pSource != sal_False);
+ *(sal_Bool *)pDestAny->pData = *(sal_Bool *)pSource;
break;
case typelib_TypeClass_BYTE:
pDestAny->pData = &pDestAny->pReserved;
Why did I get into this? Well, see https://gerrit.libreoffice.org/#/c/1164/
--
Lionel
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
