[KDE/Mac] Review Request 124894: [OS X] make it build

Thu Aug 27 10:07:24 UTC 2015

> On Aug. 27, 2015, 5:41 a.m., Kevin Ottens wrote:
> > src/domain/queryresultprovider.h, lines 35-40
> > <https://git.reviewboard.kde.org/r/124894/diff/2/?file=399196#file399196line35>
> >
> >     And don't forget to put that part in the new header. ;-)
> 
> René J.V. Bertin wrote:
>     But that's *only* required in this header, and shouldn't be included in code that doesn't load the QSharedPointer definition. I found that out the hard way, so there are that kind of files which also use mem_fn. I could of course test for the token that gets set by the headerfile defining QSharedPointer, but is user code supposed to do that kind of thing?

It would be only activated in the boost case. Also I guess there's a way to not make it dependent on QSharedPointer if that's parameterized as well. What we really want is for it to work with any smart pointer type which has a data() method to get to the raw pointer.

> On Aug. 27, 2015, 5:41 a.m., Kevin Ottens wrote:
> > src/akonadi/akonadiserializer.cpp, lines 36-41
> > <https://git.reviewboard.kde.org/r/124894/diff/2/?file=399195#file399195line36>
> >
> >     Now we have that at several places, it would be nice to have only one such block. Please put it in a header like src/utils/memfn.h
> >     
> >     I would advise turning it into a template method instead of a macro so it can properly be in a namespace, that would give calls like:
> >     Utils::memFn(&Class::method)
> 
> René J.V. Bertin wrote:
>     Go figure that I considered doing that, in the form of a private method in the class using the function in queryresultprovider.h . I didn't insist too much though, because of the fact that mem_fn has templates in both its arguments and its return types, and boost and std don't use the same syntax. That feels like a lot of work for something that is supposed to be only a temporary band-aid. At least it felt that way rather late yesterday evening when I was wrapping this up.

Private method wouldn't buy us much though. Having it in a namespace like the std and boost variant would make it properly reusable where needed.

As for it being "temporary" or not, two things:
 1) temporary things tend to stick around, so I'd rather see them done small and not spread at plenty of places;
 2) I can't judge if it'll be really temporary or not, you tell me! But it seemed important for you to support that compiler in the first place. ;-)

I guess that's the compiler most developers on OS X 10.9 will have right? Or that's common practice to upgrade it?

- Kevin

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124894/#review84439
-----------------------------------------------------------

On Aug. 26, 2015, 9:12 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124894/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 9:12 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and Zanshin.
> 
> 
> Repository: zanshin
> 
> 
> Description
> -------
> 
> This is a set of patches that allow zanshin version `0.2.1-1163-gbf5c321` to build on OS X against the git head (or almost head) of kdelibs 4.14 and kdepim* 4.14 .
> I presume for now that a number of changes are self-explanatory (for instance, `zanshinkdepimstatic` calls itself static, isn't installed and doesn't export any symbols explicitly so all points to it being intended to be a static instead of a shared lib).
> The patches that disable a number of tests, and I do not understand myself what exactly goes wrong without them other than that the linker gives errors like
> 
> ```
> # Undefined symbols for architecture x86_64:
> #   "decltype(*(std::__1::forward<QSharedPointer<Domain::QueryResult<QString> > >(fp0)).*fp(std::__1::forward<>(fp1))) std::__1::__invoke<QList<std::__1::function<void (QString, int)> > (Domain::QueryResult<QString>::*&)() const, QSharedPointer<Domain::QueryResult<QString> >, void>(QList<std::__1::function<void (QString, int)> > (Domain::QueryResult<QString>::*&&&)() const, QSharedPointer<Domain::QueryResult<QString> >&&)", referenced from:
> #       std::__1::__function::__func<std::__1::__mem_fn<QList<std::__1::function<void (QString, int)> > (Domain::QueryResult<QString>::*)() const>, std::__1::allocator<std::__1::__mem_fn<QList<std::__1::function<void (QString, int)> > (Domain::QueryResult<QString>::*)() const> >, QList<std::__1::function<void (QString, int)> > (QSharedPointer<Domain::QueryResult<QString> >)>::operator()(QSharedPointer<Domain::QueryResult<QString> >&&) in queryresulttest.o
> # ld: symbol(s) not found for architecture x86_64
> # clang: error: linker command failed with exit code 1 (use -v to see invocation)
> ```
> 
> Possibly a comparable to the reason why I had to define the `std::accumulate` functions.
> 
> 
> Diffs
> -----
> 
>   3rdparty/kdepim/libkdepim/CMakeLists.txt cc8845a 
>   3rdparty/kdepim/libkdepim/tests/CMakeLists.txt 775ac0e 
>   CMakeLists.txt 71b16a8 
>   src/akonadi/akonadiserializer.cpp 5116fa5 
>   src/domain/queryresultprovider.h 5d0b02d 
>   src/presentation/errorhandler.cpp 5610040 
>   tests/testlib/akonadistoragetestbase.cpp 8573374 
>   tests/units/akonadi/akonadidatasourcequeriestest.cpp c2e87dc 
>   tests/units/widgets/applicationcomponentstest.cpp f972673 
> 
> Diff: https://git.reviewboard.kde.org/r/124894/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with the system compiler (`Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)`)
> 
> My initial reason to install this was to "check it out" and learn how to use it, so I have not done any further testing ...
> 
> 
> File Attachments
> ----------------
> 
> datasourcequeriestest.cpp preprocessed by Apple Clang 6
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/24/b2f3e229-ca5a-4e55-b33b-6504b2cdb5c7__datasourcequeriestest-AppleClang6.i
> datasourcequeriestest.cpp preprocessed by clang 3.6.1
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/08/24/807e30a4-aa34-45b4-9e5c-92020ee8c7f5__datasourcequeriestestClang3.6.i
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20150827/174a8229/attachment-0001.html>