shouldn't XFoo implementations use sal_Bool, and not bool?

[Lionel Elie Mamane]

On Mon, Jun 01, 2015 at 09:19:40AM +0200, Stephan Bergmann wrote:
> On 05/30/2015 10:30 PM, Lionel Elie Mamane wrote:
> >          virtual ::com::sun::star::uno::Reference< ::com::sun::star::sdbc::XClob > SAL_CALL getClob( sal_Int32 columnIndex ) throw(::com::su
> >          virtual ::com::sun::star::uno::Reference< ::com::sun::star::sdbc::XArray > SAL_CALL getArray( sal_Int32 columnIndex ) throw(::com::
> >          // ::com::sun::star::sdbc::XResultSet
> >-        virtual sal_Bool SAL_CALL next(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >-        virtual sal_Bool SAL_CALL isBeforeFirst(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >-        virtual sal_Bool SAL_CALL isAfterLast(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >-        virtual sal_Bool SAL_CALL isFirst(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >-        virtual sal_Bool SAL_CALL isLast(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >+        virtual bool SAL_CALL next(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >+        virtual bool SAL_CALL isBeforeFirst(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >+        virtual bool SAL_CALL isAfterLast(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >+        virtual bool SAL_CALL isFirst(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >+        virtual bool SAL_CALL isLast(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
> >
> >
> >Which changed dbaccess/source/core/api/CacheSet.hxx to bool return
> >types. But these are (copies of) the com::sun::star::sdbc::XResultSet
> >interface, so shouldn't those have stayed sal_Bool?
> 
> The functions are apparently copied from XResultSet (incl. presumably
> unnecessary virtual and SAL_CALL, even), but technically they are unrelated.
> That means there is no technical reason to make them use the inferior
> sal_Bool instead of bool (and loplugin:salbool should warn about such
> unnecessary uses of sal_Bool).

Well, OTOH your tinderbox complains that a bool is compared to a
sal_Bool, and for functions that are copied from XResultSet putting
explicit casts is unnatural. In essence, the "cache" objects presents
the same API as the object it caches, but it just happens that all
users of the cache know they are dealing with a cache, so they don't
use the abstract base (interface) class (css::sdbc::XResultSet), but
use the "Cache" object class directly.

I think I prefer to add the inheritance from the
"css::sdbc::XResultSet" interface class, so that the things I just
explained are visible to the tools.

-- 
Lionel