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

Thu Aug 27 10:45:58 UTC 2015

> 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.
> 
> Kevin Ottens wrote:
>     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?
> 
> René J.V. Bertin wrote:
>     I suppose it also depends on the extent to which you expect the use of ::mem_fn to florish ... I'm not familiar with your code or the reasons you turned to that function, so I cannot judge this.
>     I agree that temporary code has the habit of sticking around...
>     
>     Indeed, we're talking about the system compiler on OS X 10.9, so anyone running that version of the OS won't be able to get a newer one from Apple without upgrading the OS first. It isn't hard to get a newer version from the likes of MacPorts or Fink or HomeBrew, but in my experience they tend to be up to twice as slow. Which means that I for one always try the system compiler first. (Besides, it's not like llvm + clang don't take up significant space, so if one can make do with just the system compiler that's always preferable.)
>     
>     Anyway, I'll look into your suggestion.

OK, so we can assume it won't really be temporary then...

As for mem_fn to florish, it is likely indeed. I have local patches with more uses of std::bind, and I have further plans which will require more mem_fn use. So I'd expect a serious increase.

- 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/9dea368f/attachment-0001.html>