[Kde-pim] Review Request: Polymorphic payload support for Akonadi::Item

[Thomas McGuire]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/626/#review987
-----------------------------------------------------------

Ship it!


Looks nice, although the advanced template and type trait stuff is a bit above me. But it has good documentation, so not a problem.
Commit it!


/trunk/KDE/kdepimlibs/akonadi/exception.cpp
<http://reviewboard.kde.org/r/626/#comment667>

    Do we need this member? It is only used in what(), and there a local var could be used as well, right?


- Thomas


On None, Volker Krause wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/626/
> -----------------------------------------------------------
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Polymorphic payload support means that the payload mechanism in Akonadi::Item is no longer limited to retrieve the exact same payload type that was put in, but can now also cast if possible, which is especially useful with payloads like KCal or KMime objects.
> 
> However, implementing that is much harder than it sounds and requires some external hints, namely compile-time introspection of the inheritance hierarchy. This is provided by KPIMUtils::SuperTrait and its specializations in KCal and KMime. This is pure template stuff, so no effects on the ABI. As it is needed by Akonadi, KCal and KMime, kpimutils was unfortunately the only possible location, apart from a new top-level directory.
> 
> While I was at it I also added convenience typedefs for shared pointers for KCal and KMime objects, as we have them all over Akonadi code currently. They only use forward declarations, so no there is no new boost dependency for those libraries.
> 
> The changes to Akonadi::Item itself have a few side-effects though, besides adding support for polymorphic payloads in boost::shared_ptr and QSharedPointer:
> - There are strict compile-time checks for invalid payload types now, so trying to use a pointer as payload will fail to compile instead of crash at runtime now.
> - When using a shared pointer payload, the class the shared pointer contains now needs to be fully declared when the payload related templates are instantiated, so far one could get away with just a forward declaration in some cases. I only found one occurence of that in our code, so this doesn't seem to be a problem in practice.
> - So far behaviour was pretty much undefined when retrieving a mismatching payload type (most probably it would crash somehow), in this cases an exception is thrown now. This will reliably crash if the application doesn't care (normal code should use hasPayload() and never get there), but also allows handling this kind of error (which eg. the Item code uses internally). This however has one big downside: we need to compile every user of the payload mechanism with exception support now (the exception throwing happens in template code). One could argue this is a SIC change, in fact multiple applications in kdepim needed ${KDE4_ENABLE_EXCEPTIONS} added in their cmake files.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepimlibs/akonadi/CMakeLists.txt 956053 
>   /trunk/KDE/kdepimlibs/akonadi/exception.h PRE-CREATION 
>   /trunk/KDE/kdepimlibs/akonadi/exception.cpp PRE-CREATION 
>   /trunk/KDE/kdepimlibs/akonadi/item.h 956053 
>   /trunk/KDE/kdepimlibs/akonadi/itempayloadinternals_p.h 956053 
>   /trunk/KDE/kdepimlibs/akonadi/tests/itemhydratest.h 956053 
>   /trunk/KDE/kdepimlibs/akonadi/tests/itemhydratest.cpp 956053 
>   /trunk/KDE/kdepimlibs/akonadi/tests/testrunner/feeditem.cpp 956053 
>   /trunk/KDE/kdepimlibs/kcal/event.h 956053 
>   /trunk/KDE/kdepimlibs/kcal/incidence.h 956053 
>   /trunk/KDE/kdepimlibs/kcal/journal.h 956053 
>   /trunk/KDE/kdepimlibs/kcal/todo.h 956053 
>   /trunk/KDE/kdepimlibs/kmime/kmime_message.h 956053 
>   /trunk/KDE/kdepimlibs/kmime/kmime_newsarticle.h 956053 
>   /trunk/KDE/kdepimlibs/kpimutils/CMakeLists.txt 956053 
>   /trunk/KDE/kdepimlibs/kpimutils/supertrait.h PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/626/diff
> 
> 
> Testing
> -------
> 
> Run and extended unit tests.
> 
> 
> Thanks,
> 
> Volker
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/