No UNSIGNED_BYTE in Any

[Lionel Elie Mamane]

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